diff --git a/test/vectors/s3vectors_test.py b/test/vectors/s3vectors_test.py index a6857b13ef..f9866d7a25 100644 --- a/test/vectors/s3vectors_test.py +++ b/test/vectors/s3vectors_test.py @@ -4,8 +4,11 @@ Run via `make test` — a mini cluster, an s3 gateway serving the vectors API on weed-vector are up by then. """ +import json + import boto3 import pytest +from botocore.exceptions import ClientError ENDPOINT = "http://127.0.0.1:8334" BUCKET = "vectors-bucket" @@ -79,3 +82,31 @@ def test_vector_lifecycle(vectors): vectors.delete_index(vectorBucketName=BUCKET, indexName=INDEX) vectors.delete_vector_bucket(vectorBucketName=BUCKET) + + +def test_vector_bucket_policy(vectors): + vectors.create_vector_bucket(vectorBucketName=BUCKET) + try: + policy = json.dumps( + { + "Version": "2012-10-17", + "Statement": [ + { + "Effect": "Allow", + "Principal": {"AWS": "*"}, + "Action": "s3tables:*", + "Resource": "*", + } + ], + } + ) + vectors.put_vector_bucket_policy(vectorBucketName=BUCKET, policy=policy) + got = vectors.get_vector_bucket_policy(vectorBucketName=BUCKET)["policy"] + assert json.loads(got) == json.loads(policy) + + vectors.delete_vector_bucket_policy(vectorBucketName=BUCKET) + with pytest.raises(ClientError) as exc: + vectors.get_vector_bucket_policy(vectorBucketName=BUCKET) + assert exc.value.response["Error"]["Code"] == "NotFoundException" + finally: + vectors.delete_vector_bucket(vectorBucketName=BUCKET) diff --git a/weed/s3api/s3tables/handler_policy.go b/weed/s3api/s3tables/handler_policy.go index c152b209a9..c6c6002a7f 100644 --- a/weed/s3api/s3tables/handler_policy.go +++ b/weed/s3api/s3tables/handler_policy.go @@ -60,6 +60,11 @@ func (h *S3TablesHandler) handlePutTableBucketPolicy(w http.ResponseWriter, r *h return fmt.Errorf("resourcePolicy is required") } + if err := validatePolicyDocument(req.ResourcePolicy); err != nil { + h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) + return err + } + bucketName, err := parseBucketNameFromARN(req.TableBucketARN) if err != nil { h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) @@ -320,6 +325,11 @@ func (h *S3TablesHandler) handlePutTablePolicy(w http.ResponseWriter, r *http.Re return fmt.Errorf("resourcePolicy is required") } + if err := validatePolicyDocument(req.ResourcePolicy); err != nil { + h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) + return err + } + bucketName, err := parseBucketNameFromARN(req.TableBucketARN) if err != nil { h.writeError(w, http.StatusBadRequest, ErrCodeInvalidRequest, err.Error()) diff --git a/weed/s3api/s3tables/permissions.go b/weed/s3api/s3tables/permissions.go index 28e9a3dcf3..dee5fa3eb2 100644 --- a/weed/s3api/s3tables/permissions.go +++ b/weed/s3api/s3tables/permissions.go @@ -141,6 +141,41 @@ func CheckTableOrBucketPermission(operation, principal, resourceOwner, resourceP return resourceOK || bucketOK } +// validatePolicyDocument rejects a policy the evaluator could not honor: one +// that does not parse, or whose actions live in a namespace that never +// matches. An s3vectors action stored here would be a dead rule the writer +// believes is enforced — vector requests authorize their s3tables action. +func validatePolicyDocument(raw string) error { + var doc PolicyDocument + if err := json.Unmarshal([]byte(raw), &doc); err != nil { + return fmt.Errorf("invalid policy document: %w", err) + } + for _, stmt := range doc.Statement { + for _, action := range statementActions(stmt.Action) { + if strings.HasPrefix(action, "s3vectors:") { + return fmt.Errorf("action %q is not enforceable in a resource policy", action) + } + } + } + return nil +} + +func statementActions(spec interface{}) []string { + switch a := spec.(type) { + case string: + return []string{a} + case []interface{}: + var actions []string + for _, item := range a { + if str, ok := item.(string); ok { + actions = append(actions, str) + } + } + return actions + } + return nil +} + // deniedByPolicy reports whether another applicable resource policy vetoes the // request. The admin account is exempt, matching CheckPermissionWithContext. func deniedByPolicy(operation, principal, resourcePolicy, resourceARN string, ctx *PolicyContext) bool { diff --git a/weed/s3api/s3tables/permissions_test.go b/weed/s3api/s3tables/permissions_test.go index 62f67deabd..4496cab7cd 100644 --- a/weed/s3api/s3tables/permissions_test.go +++ b/weed/s3api/s3tables/permissions_test.go @@ -428,3 +428,21 @@ func TestCheckPermissionLegacyPolicyPrincipalShapes(t *testing.T) { t.Error("legacy Allow with array principal must still match the scoped caller") } } + +func TestValidatePolicyDocument(t *testing.T) { + valid := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3tables:GetTable","Resource":"*"}]}` + if err := validatePolicyDocument(valid); err != nil { + t.Fatalf("valid policy rejected: %v", err) + } + if err := validatePolicyDocument("not-json"); err == nil { + t.Fatal("malformed policy should be rejected") + } + dead := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":"s3vectors:PutVectors","Resource":"*"}]}` + if err := validatePolicyDocument(dead); err == nil { + t.Fatal("s3vectors action should be rejected") + } + deadList := `{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","Action":["s3tables:*","s3vectors:*"],"Resource":"*"}]}` + if err := validatePolicyDocument(deadList); err == nil { + t.Fatal("action list containing s3vectors should be rejected") + } +} diff --git a/weed/s3api/s3vectors/handlers.go b/weed/s3api/s3vectors/handlers.go index 8a627a486a..791d9ad983 100644 --- a/weed/s3api/s3vectors/handlers.go +++ b/weed/s3api/s3vectors/handlers.go @@ -20,19 +20,22 @@ import ( ) var handlers = map[string]func(*Server, http.ResponseWriter, *http.Request){ - "CreateVectorBucket": (*Server).handleCreateVectorBucket, - "GetVectorBucket": (*Server).handleGetVectorBucket, - "ListVectorBuckets": (*Server).handleListVectorBuckets, - "DeleteVectorBucket": (*Server).handleDeleteVectorBucket, - "CreateIndex": (*Server).handleCreateIndex, - "GetIndex": (*Server).handleGetIndex, - "ListIndexes": (*Server).handleListIndexes, - "DeleteIndex": (*Server).handleDeleteIndex, - "PutVectors": (*Server).handlePutVectors, - "GetVectors": (*Server).handleGetVectors, - "ListVectors": (*Server).handleListVectors, - "DeleteVectors": (*Server).handleDeleteVectors, - "QueryVectors": (*Server).handleQueryVectors, + "CreateVectorBucket": (*Server).handleCreateVectorBucket, + "GetVectorBucket": (*Server).handleGetVectorBucket, + "ListVectorBuckets": (*Server).handleListVectorBuckets, + "DeleteVectorBucket": (*Server).handleDeleteVectorBucket, + "PutVectorBucketPolicy": (*Server).handlePutVectorBucketPolicy, + "GetVectorBucketPolicy": (*Server).handleGetVectorBucketPolicy, + "DeleteVectorBucketPolicy": (*Server).handleDeleteVectorBucketPolicy, + "CreateIndex": (*Server).handleCreateIndex, + "GetIndex": (*Server).handleGetIndex, + "ListIndexes": (*Server).handleListIndexes, + "DeleteIndex": (*Server).handleDeleteIndex, + "PutVectors": (*Server).handlePutVectors, + "GetVectors": (*Server).handleGetVectors, + "ListVectors": (*Server).handleListVectors, + "DeleteVectors": (*Server).handleDeleteVectors, + "QueryVectors": (*Server).handleQueryVectors, } // lance table ids put indexes under a "vector" namespace inside their bucket. @@ -79,6 +82,25 @@ func arnParts(v *vectorBucketARNs, name, arn string) error { return nil } +// bucketARNs resolves the bucket a bucket-level request names and rejects +// forms that point at a different resource: an index ARN names a table, and an +// ARN minted for another service names that service's bucket. +func bucketARNs(name, arn string) (vectorBucketARNs, error) { + var v vectorBucketARNs + if err := arnParts(&v, name, arn); err != nil { + return v, err + } + if v.index != "" { + return v, fmt.Errorf("vectorBucketArn %q names an index, not a bucket", arn) + } + if strings.HasPrefix(arn, "arn:") { + if p := strings.SplitN(arn, ":", 6); len(p) > 2 && p[2] != "s3vectors" { + return v, fmt.Errorf("vectorBucketArn %q is not an s3vectors ARN", arn) + } + } + return v, nil +} + // resolveIndexARNs fills bucket and index from any combination of plain names, // a bucket ARN, and a full index ARN — callers may supply only an indexArn. func resolveIndexARNs(bucketName, bucketArn, indexName, indexArn string) (vectorBucketARNs, error) { @@ -276,13 +298,13 @@ func (s *Server) handleGetVectorBucket(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } - var arns vectorBucketARNs - if err := arnParts(&arns, req.VectorBucketName, req.VectorBucketArn); err != nil { + arns, err := bucketARNs(req.VectorBucketName, req.VectorBucketArn) + if err != nil { writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } var resp map[string]interface{} - err := s.catalog(r.Context(), "GetTableBucket", + err = s.catalog(r.Context(), "GetTableBucket", &s3tables.GetTableBucketRequest{TableBucketARN: s.s3tablesArn(arns.bucket)}, &resp, r) if err != nil { catalogErr(w, err) @@ -348,8 +370,8 @@ func (s *Server) handleDeleteVectorBucket(w http.ResponseWriter, r *http.Request writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } - var arns vectorBucketARNs - if err := arnParts(&arns, req.VectorBucketName, req.VectorBucketArn); err != nil { + arns, err := bucketARNs(req.VectorBucketName, req.VectorBucketArn) + if err != nil { writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } @@ -360,7 +382,7 @@ func (s *Server) handleDeleteVectorBucket(w http.ResponseWriter, r *http.Request return } var resp map[string]interface{} - err := s.catalog(r.Context(), "DeleteTableBucket", + err = s.catalog(r.Context(), "DeleteTableBucket", &s3tables.DeleteTableBucketRequest{TableBucketARN: s.s3tablesArn(arns.bucket)}, &resp, r) if err != nil && catalogErrCode(err) == s3tables.ErrCodeBucketNotEmpty { // The implicit index namespace counts as bucket content even when it @@ -402,6 +424,87 @@ func tableID(bucket, index string) *vector_pb.Table { return &vector_pb.Table{Id: []string{bucket, indexNamespace, index}} } +// Vector bucket policies are stored on the shared table bucket, the same +// resource s3tables' PutTableBucketPolicy guards. +func (s *Server) handlePutVectorBucketPolicy(w http.ResponseWriter, r *http.Request) { + var req PutVectorBucketPolicyRequest + if err := readJSON(r, &req); err != nil { + writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) + return + } + arns, err := bucketARNs(req.VectorBucketName, req.VectorBucketArn) + if err != nil { + writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) + return + } + if err := s.lanceBucket(r.Context(), arns.bucket); err != nil { + catalogErr(w, err) + return + } + var resp map[string]interface{} + err = s.catalog(r.Context(), "PutTableBucketPolicy", &s3tables.PutTableBucketPolicyRequest{ + TableBucketARN: s.s3tablesArn(arns.bucket), + ResourcePolicy: req.Policy, + }, &resp, r) + if err != nil { + catalogErr(w, err) + return + } + writeJSON(w, http.StatusOK, &PutVectorBucketPolicyResponse{}) +} + +func (s *Server) handleGetVectorBucketPolicy(w http.ResponseWriter, r *http.Request) { + var req GetVectorBucketPolicyRequest + if err := readJSON(r, &req); err != nil { + writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) + return + } + arns, err := bucketARNs(req.VectorBucketName, req.VectorBucketArn) + if err != nil { + writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) + return + } + if err := s.lanceBucket(r.Context(), arns.bucket); err != nil { + catalogErr(w, err) + return + } + var resp s3tables.GetTableBucketPolicyResponse + err = s.catalog(r.Context(), "GetTableBucketPolicy", &s3tables.GetTableBucketPolicyRequest{ + TableBucketARN: s.s3tablesArn(arns.bucket), + }, &resp, r) + if err != nil { + catalogErr(w, err) + return + } + writeJSON(w, http.StatusOK, &GetVectorBucketPolicyResponse{Policy: resp.ResourcePolicy}) +} + +func (s *Server) handleDeleteVectorBucketPolicy(w http.ResponseWriter, r *http.Request) { + var req DeleteVectorBucketPolicyRequest + if err := readJSON(r, &req); err != nil { + writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) + return + } + arns, err := bucketARNs(req.VectorBucketName, req.VectorBucketArn) + if err != nil { + writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) + return + } + if err := s.lanceBucket(r.Context(), arns.bucket); err != nil { + catalogErr(w, err) + return + } + var resp map[string]interface{} + err = s.catalog(r.Context(), "DeleteTableBucketPolicy", &s3tables.DeleteTableBucketPolicyRequest{ + TableBucketARN: s.s3tablesArn(arns.bucket), + }, &resp, r) + if err != nil { + catalogErr(w, err) + return + } + writeJSON(w, http.StatusOK, &DeleteVectorBucketPolicyResponse{}) +} + func (s *Server) handleCreateIndex(w http.ResponseWriter, r *http.Request) { var req CreateIndexRequest if err := readJSON(r, &req); err != nil { @@ -608,8 +711,8 @@ func (s *Server) handleListIndexes(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } - var arns vectorBucketARNs - if err := arnParts(&arns, req.VectorBucketName, req.VectorBucketArn); err != nil { + arns, err := bucketARNs(req.VectorBucketName, req.VectorBucketArn) + if err != nil { writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } @@ -621,7 +724,7 @@ func (s *Server) handleListIndexes(w http.ResponseWriter, r *http.Request) { } `json:"tables"` ContinuationToken string `json:"continuationToken"` } - err := s.catalog(r.Context(), "ListTables", &s3tables.ListTablesRequest{ + err = s.catalog(r.Context(), "ListTables", &s3tables.ListTablesRequest{ TableBucketARN: s.s3tablesArn(arns.bucket), Namespace: []string{indexNamespace}, MaxTables: req.MaxResults, diff --git a/weed/s3api/s3vectors/handlers_test.go b/weed/s3api/s3vectors/handlers_test.go index 542327e66b..214c8c9dce 100644 --- a/weed/s3api/s3vectors/handlers_test.go +++ b/weed/s3api/s3vectors/handlers_test.go @@ -202,6 +202,24 @@ func TestArnParts(t *testing.T) { } } +func TestBucketARNs(t *testing.T) { + if v, err := bucketARNs("plain", ""); err != nil || v.bucket != "plain" { + t.Fatalf("plain name: %+v err=%v", v, err) + } + if v, err := bucketARNs("", "arn:aws:s3vectors:r:a:bucket/mybucket"); err != nil || v.bucket != "mybucket" { + t.Fatalf("bucket ARN: %+v err=%v", v, err) + } + for _, bad := range []string{ + "arn:aws:s3vectors:r:a:bucket/b/index/i", + "arn:aws:s3tables:r:a:bucket/b", + "arn:aws:s3:r:a:bucket/b", + } { + if _, err := bucketARNs("", bad); err == nil { + t.Fatalf("ARN %q should be rejected for a bucket operation", bad) + } + } +} + func TestResolveIndexARNs(t *testing.T) { arns, err := resolveIndexARNs("", "", "", "arn:aws:s3vectors:r:a:bucket/b/index/i") if err != nil || arns.bucket != "b" || arns.index != "i" { diff --git a/weed/s3api/s3vectors/types.go b/weed/s3api/s3vectors/types.go index f007262fbf..36b474eb45 100644 --- a/weed/s3api/s3vectors/types.go +++ b/weed/s3api/s3vectors/types.go @@ -94,6 +94,30 @@ type DeleteVectorBucketRequest struct { type DeleteVectorBucketResponse struct{} +type PutVectorBucketPolicyRequest struct { + VectorBucketName string `json:"vectorBucketName"` + VectorBucketArn string `json:"vectorBucketArn"` + Policy string `json:"policy"` +} + +type PutVectorBucketPolicyResponse struct{} + +type GetVectorBucketPolicyRequest struct { + VectorBucketName string `json:"vectorBucketName"` + VectorBucketArn string `json:"vectorBucketArn"` +} + +type GetVectorBucketPolicyResponse struct { + Policy string `json:"policy"` +} + +type DeleteVectorBucketPolicyRequest struct { + VectorBucketName string `json:"vectorBucketName"` + VectorBucketArn string `json:"vectorBucketArn"` +} + +type DeleteVectorBucketPolicyResponse struct{} + type CreateIndexRequest struct { DataType string `json:"dataType"` Dimension int32 `json:"dimension"`