From 8125a1549aa44cc3886c790351de17a2337b2d03 Mon Sep 17 00:00:00 2001 From: Chris Lu Date: Mon, 2 Nov 2026 12:43:12 -0800 Subject: [PATCH] s3vectors: support tag conditions in vector bucket policies (#12351) * s3tables: resolve s3vectors:VectorBucketTag like s3tables:TableBucketTag Both name a tag on the bucket that holds the target, whichever resource the request acts on. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * s3vectors: give vector policy conditions the bucket, index and request tags A vector bucket policy is evaluated with these condition keys: - aws:ResourceTag/: the tags of the bucket, or of the index when the request names one. - s3vectors:VectorBucketTag/: the bucket's tags, for bucket and index requests alike. - aws:RequestTag/ and aws:TagKeys: the tags TagResource and CreateIndex set, and the keys UntagResource removes. An index's tags are read only when the policy has an aws:ResourceTag condition. The bucket's come from the lookup that already reads its policy. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * s3vectors: accept tag conditions in vector bucket policies PutVectorBucketPolicy rejected every Condition, since no condition key was supplied. A condition is now accepted when its operator is one the evaluator knows and each key is aws:ResourceTag/, aws:RequestTag/, aws:TagKeys or s3vectors:VectorBucketTag/. Any other key would never match, so it is still rejected, as is a condition field not spelled Condition. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * test/s3vectors: cover tag conditions in vector bucket policies Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK * s3vectors: reject condition values the evaluator cannot compare A null condition value was accepted and then panicked the request that matched its statement, a Deny reaching even the bucket owner. A value must now be a string, number or boolean, or a non-empty list of them. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_011UpvafvK8JpagF94MHWqwK --------- Co-authored-by: Claude Opus 5.5 --- test/s3vectors/s3vectors_test.py | 41 +++ weed/s3api/s3tables/permissions.go | 9 +- weed/s3api/s3tables/permissions_test.go | 19 ++ weed/s3api/s3vectors/handlers.go | 203 +++++++++++-- weed/s3api/s3vectors/handlers_test.go | 2 +- weed/s3api/s3vectors/tag_conditions_test.go | 316 ++++++++++++++++++++ weed/s3api/s3vectors/tagging.go | 10 +- 7 files changed, 562 insertions(+), 38 deletions(-) create mode 100644 weed/s3api/s3vectors/tag_conditions_test.go diff --git a/test/s3vectors/s3vectors_test.py b/test/s3vectors/s3vectors_test.py index 05f83943ee..375e2d6242 100644 --- a/test/s3vectors/s3vectors_test.py +++ b/test/s3vectors/s3vectors_test.py @@ -451,6 +451,47 @@ def test_create_with_tags(vectors): vectors.delete_vector_bucket(vectorBucketName=bucket) +def test_tag_conditions(vectors, vector_clients, index): + reader, writer = vector_clients["reader"], vector_clients["writer"] + bucket = index["vectorBucketName"] + bucket_arn = vectors.get_vector_bucket(vectorBucketName=bucket)["vectorBucket"]["vectorBucketArn"] + index_arn = vectors.get_index(**index)["index"]["indexArn"] + other = dict(index, indexName="other-index") + vectors.create_index(**other, dataType="float32", dimension=4, distanceMetric="euclidean") + vectors.tag_resource(resourceArn=bucket_arn, tags={"env": "prod"}) + vectors.tag_resource(resourceArn=index_arn, tags={"stage": "live"}) + indexes = bucket_arn + "/index/*" + statements = [ + {"Effect": "Allow", "Principal": {"AWS": "reader"}, "Action": "s3vectors:GetIndex", "Resource": indexes, + "Condition": {"StringEquals": {"aws:ResourceTag/stage": "live"}}}, + {"Effect": "Allow", "Principal": {"AWS": "reader"}, "Action": "s3vectors:QueryVectors", "Resource": indexes, + "Condition": {"StringEquals": {"s3vectors:VectorBucketTag/env": "prod"}}}, + {"Effect": "Allow", "Principal": {"AWS": "writer"}, "Action": "s3vectors:TagResource", "Resource": indexes, + "Condition": {"StringEquals": {"aws:RequestTag/team": "search"}, + "ForAllValues:StringEquals": {"aws:TagKeys": ["team"]}}}, + ] + try: + vectors.put_vector_bucket_policy(vectorBucketName=bucket, + policy=json.dumps({"Version": "2012-10-17", "Statement": statements})) + assert reader.get_index(**index)["index"]["indexArn"] == index_arn + expect_error("AccessDeniedException", reader.get_index, **other) + assert reader.query_vectors(**other, queryVector={"float32": [1, 0, 0, 0]}, topK=1)["vectors"] == [] + writer.tag_resource(resourceArn=index_arn, tags={"team": "search"}) + expect_error("AccessDeniedException", writer.tag_resource, resourceArn=index_arn, tags={"team": "other"}) + expect_error("AccessDeniedException", writer.tag_resource, resourceArn=index_arn, + tags={"team": "search", "env": "dev"}) + assert vectors.list_tags_for_resource(resourceArn=index_arn)["tags"] == {"stage": "live", "team": "search"} + vectors.tag_resource(resourceArn=bucket_arn, tags={"env": "dev"}) + expect_error("AccessDeniedException", reader.query_vectors, **other, + queryVector={"float32": [1, 0, 0, 0]}, topK=1) + unsupported = dict(statements[0], Condition={"IpAddress": {"aws:SourceIp": "10.0.0.0/8"}}) + expect_error("ValidationException", vectors.put_vector_bucket_policy, vectorBucketName=bucket, + policy=json.dumps({"Version": "2012-10-17", "Statement": [unsupported]})) + finally: + vectors.delete_vector_bucket_policy(vectorBucketName=bucket) + vectors.delete_index(**other) + + def test_invalid_credentials(cluster): client = boto3.client("s3vectors", endpoint_url=cluster["endpoint"], aws_access_key_id="unknown", aws_secret_access_key="unknown", region_name="us-east-1") diff --git a/weed/s3api/s3tables/permissions.go b/weed/s3api/s3tables/permissions.go index 3cbd3a55d7..2184668776 100644 --- a/weed/s3api/s3tables/permissions.go +++ b/weed/s3api/s3tables/permissions.go @@ -586,9 +586,12 @@ func getConditionContextValues(key string, ctx *PolicyContext) []string { return []string{val} } } - if strings.HasPrefix(key, "s3tables:TableBucketTag/") { - tagKey := strings.TrimPrefix(key, "s3tables:TableBucketTag/") - if val, ok := ctx.TableBucketTags[tagKey]; ok { + tagKey, ok := strings.CutPrefix(key, "s3tables:TableBucketTag/") + if !ok { + tagKey, ok = strings.CutPrefix(key, "s3vectors:VectorBucketTag/") + } + if ok { + if val, found := ctx.TableBucketTags[tagKey]; found { return []string{val} } } diff --git a/weed/s3api/s3tables/permissions_test.go b/weed/s3api/s3tables/permissions_test.go index f2dabcda8e..037cdbb2ce 100644 --- a/weed/s3api/s3tables/permissions_test.go +++ b/weed/s3api/s3tables/permissions_test.go @@ -538,3 +538,22 @@ func TestServiceWildcardScopesToOwnService(t *testing.T) { t.Error("the global admin action keeps the bypass on vector operations") } } + +func TestBucketTagConditionKeys(t *testing.T) { + ctx := &PolicyContext{ + TableBucketTags: map[string]string{"env": "prod"}, + ResourceTags: map[string]string{"env": "dev"}, + } + for key, want := range map[string]string{ + "s3tables:TableBucketTag/env": "prod", + "s3vectors:VectorBucketTag/env": "prod", + "aws:ResourceTag/env": "dev", + } { + if got := getConditionContextValues(key, ctx); len(got) != 1 || got[0] != want { + t.Errorf("%s = %v, want %s", key, got, want) + } + } + if got := getConditionContextValues("s3vectors:VectorBucketTag/team", ctx); got != nil { + t.Errorf("absent bucket tag = %v, want none", got) + } +} diff --git a/weed/s3api/s3vectors/handlers.go b/weed/s3api/s3vectors/handlers.go index 0f629d2cc1..edd6609711 100644 --- a/weed/s3api/s3vectors/handlers.go +++ b/weed/s3api/s3vectors/handlers.go @@ -5,8 +5,10 @@ import ( "encoding/json" "errors" "fmt" + "maps" "math" "net/http" + "slices" "strconv" "strings" "time" @@ -16,6 +18,7 @@ import ( "github.com/seaweedfs/seaweedfs/weed/glog" "github.com/seaweedfs/seaweedfs/weed/pb/filer_pb" "github.com/seaweedfs/seaweedfs/weed/pb/vector_pb" + "github.com/seaweedfs/seaweedfs/weed/s3api/policy_engine" "github.com/seaweedfs/seaweedfs/weed/s3api/s3tables" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" @@ -385,14 +388,14 @@ func (s *Server) handleListVectorBuckets(w http.ResponseWriter, r *http.Request) if !IsVectorBucketFormat(b.Format) { continue } - owner, policy, err := s.vectorPolicyView(r.Context(), b.Name) + owner, policy, tags, err := s.vectorPolicyView(r.Context(), b.Name) if err != nil { // A bucket that vanished mid-list drops out of the page rather // than failing the whole listing. continue } arn := s.vectorBucketArn(b.Name) - pctx := s.tables.PolicyContext(r, b.Name) + pctx := s.policyContext(r, b.Name, tags, tags, tagRequest{}) if !s3tables.CheckPermissionWithContext("s3vectors:ListVectorBuckets", principal, owner, policy, arn, pctx) && !s3tables.CheckPermissionWithContext("s3vectors:GetVectorBucket", principal, owner, policy, arn, pctx) { continue @@ -484,10 +487,10 @@ var errNoVectorPolicy = errors.New("vector bucket policy not found") // validateVectorPolicy rejects a document the evaluator cannot honor: the // shared Statement shape has no negated fields, so a rule relying on one would -// silently lose its exclusion; conditions are unsupported since the vector -// evaluation supplies no condition keys; and a rule naming another service is -// a dead rule the writer believes is enforced. The document must decode into -// the same typed form the evaluator uses. +// silently lose its exclusion; a condition on a key the vector evaluation does +// not supply, or with an operator it does not know, never matches; and a rule +// naming another service is a dead rule the writer believes is enforced. The +// document must decode into the same typed form the evaluator uses. func validateVectorPolicy(raw string) error { var typed s3tables.PolicyDocument if err := json.Unmarshal([]byte(raw), &typed); err != nil { @@ -499,14 +502,30 @@ func validateVectorPolicy(raw string) error { } for _, stmt := range policyStatements(doc) { for key := range stmt { - for _, field := range []string{"NotAction", "NotResource", "NotPrincipal", "Condition"} { + for _, field := range []string{"NotAction", "NotResource", "NotPrincipal"} { if strings.EqualFold(key, field) { return fmt.Errorf("invalid policy document: %s is not supported", field) } } + if key != "Condition" && strings.EqualFold(key, "Condition") { + return fmt.Errorf("invalid policy document: %s must be spelled Condition", key) + } } } for _, stmt := range typed.Statement { + for operator, keys := range stmt.Condition { + if _, err := policy_engine.GetConditionEvaluator(operator); err != nil { + return fmt.Errorf("invalid policy document: condition operator %q is not supported", operator) + } + for key, value := range keys { + if !vectorConditionKey(key) { + return fmt.Errorf("invalid policy document: condition key %q is not supported in a vector bucket policy", key) + } + if !conditionValue(value) { + return fmt.Errorf("invalid policy document: condition %s on %q needs a string, number or boolean, or a non-empty list of them", operator, key) + } + } + } for _, action := range statementActions(stmt.Action) { if service, _, found := strings.Cut(action, ":"); found && service != "s3vectors" { return fmt.Errorf("invalid policy document: action %q is not enforceable in a vector bucket policy", action) @@ -521,6 +540,36 @@ func validateVectorPolicy(raw string) error { return nil } +// conditionValue reports whether the evaluator can compare v: a null value +// would panic it, and an empty list never matches. +func conditionValue(v interface{}) bool { + list, ok := v.([]interface{}) + if !ok { + list = []interface{}{v} + } + for _, item := range list { + switch item.(type) { + case string, float64, bool: + default: + return false + } + } + return len(list) > 0 +} + +// vectorConditionKey reports whether the vector evaluation supplies key. +func vectorConditionKey(key string) bool { + if key == "aws:TagKeys" { + return true + } + for _, prefix := range []string{"aws:ResourceTag/", "aws:RequestTag/", "s3vectors:VectorBucketTag/"} { + if tagKey, ok := strings.CutPrefix(key, prefix); ok { + return tagKey != "" + } + } + return false +} + // arnService returns the service field of an ARN in any partition. func arnService(resource string) string { parts := strings.SplitN(resource, ":", 4) @@ -672,14 +721,13 @@ func (s *Server) vectorBucket(ctx context.Context, bucket string) (*s3tables.Get return &resp, nil } -// vectorPolicy reads the bucket's stored s3vectors policy. A bucket written -// before the dedicated key carried the raw document in the shared policy -// slot; one that still names vector actions keeps working until the next -// vector policy write migrates it out. -func (s *Server) vectorPolicy(ctx context.Context, bucket string) (string, error) { +// vectorPolicy reads the bucket's stored s3vectors policy and its tags. A +// bucket written before the dedicated key carried the raw document in the +// shared policy slot; one that still names vector actions keeps working until +// the next vector policy write migrates it out. +func (s *Server) vectorPolicy(ctx context.Context, bucket string) (policy string, tags map[string]string, err error) { dir, name := splitEntryPath(s3tables.GetTableBucketPath(bucket)) - var policy string - err := s.filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { + err = s.filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { resp, err := filer_pb.LookupEntry(ctx, client, &filer_pb.LookupDirectoryEntryRequest{ Directory: dir, Name: name, @@ -688,14 +736,94 @@ func (s *Server) vectorPolicy(ctx context.Context, bucket string) (string, error return err } policy = vectorPolicyFromExt(resp.Entry.Extended) - return nil + tags, err = tagsFromExt(resp.Entry.Extended) + return err }) - return policy, err + return policy, tags, err } -// vectorPolicyView resolves a bucket's owner and stored s3vectors policy in -// one filer lookup — the per-bucket check the vector listing runs. -func (s *Server) vectorPolicyView(ctx context.Context, bucket string) (owner, policy string, err error) { +// indexTags reads an index's tags; one not created yet has none. +func (s *Server) indexTags(ctx context.Context, bucket, index string) (map[string]string, error) { + dir, name := splitEntryPath(s3tables.GetTablePath(bucket, indexNamespace, index)) + var tags map[string]string + err := s.filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { + resp, err := filer_pb.LookupEntry(ctx, client, &filer_pb.LookupDirectoryEntryRequest{ + Directory: dir, + Name: name, + }) + if errors.Is(err, filer_pb.ErrNotFound) { + return nil + } + if err != nil { + return err + } + tags, err = tagsFromExt(resp.Entry.Extended) + return err + }) + return tags, err +} + +func tagsFromExt(ext map[string][]byte) (map[string]string, error) { + raw := ext[s3tables.ExtendedKeyTags] + if len(raw) == 0 { + return nil, nil + } + var tags map[string]string + if err := json.Unmarshal(raw, &tags); err != nil { + return nil, fmt.Errorf("decode tags: %w", err) + } + return tags, nil +} + +// readsResourceTags reports whether a policy has a condition on +// aws:ResourceTag, which on an index needs the index's tags read. A document +// that does not decode is treated as having one. +func readsResourceTags(policy string) bool { + if policy == "" { + return false + } + var doc s3tables.PolicyDocument + if json.Unmarshal([]byte(policy), &doc) != nil { + return true + } + for _, stmt := range doc.Statement { + for _, keys := range stmt.Condition { + for key := range keys { + if strings.HasPrefix(key, "aws:ResourceTag/") { + return true + } + } + } + } + return false +} + +// tagRequest is what a request sets or removes, for aws:RequestTag and +// aws:TagKeys. +type tagRequest struct { + tags map[string]string + keys []string +} + +func setTags(tags map[string]string) tagRequest { + return tagRequest{tags: tags, keys: slices.Sorted(maps.Keys(tags))} +} + +// policyContext is the catalog's evaluation context plus the tag condition +// keys: aws:ResourceTag reads the target's tags and s3vectors:VectorBucketTag +// the bucket's. +func (s *Server) policyContext(r *http.Request, bucket string, bucketTags, resourceTags map[string]string, req tagRequest) *s3tables.PolicyContext { + pctx := s.tables.PolicyContext(r, bucket) + pctx.TableBucketTags = bucketTags + pctx.ResourceTags = resourceTags + pctx.RequestTags = req.tags + pctx.TagKeys = req.keys + return pctx +} + +// vectorPolicyView resolves a bucket's owner, stored s3vectors policy and +// tags in one filer lookup — the per-bucket check the vector listing runs. +func (s *Server) vectorPolicyView(ctx context.Context, bucket string) (owner, policy string, tags map[string]string, err error) { dir, name := splitEntryPath(s3tables.GetTableBucketPath(bucket)) err = s.filerClient.WithFilerClient(false, func(client filer_pb.SeaweedFilerClient) error { resp, err := filer_pb.LookupEntry(ctx, client, &filer_pb.LookupDirectoryEntryRequest{ @@ -713,9 +841,10 @@ func (s *Server) vectorPolicyView(ctx context.Context, bucket string) (owner, po } owner = meta.OwnerAccountID policy = vectorPolicyFromExt(resp.Entry.Extended) - return nil + tags, err = tagsFromExt(resp.Entry.Extended) + return err }) - return owner, policy, err + return owner, policy, tags, err } // mutateVectorPolicy applies a change to the bucket's vector-policy @@ -787,9 +916,13 @@ func (s *Server) checkVectorPolicyDoc(r *http.Request, action, bucket string, re if json.Unmarshal(ext[s3tables.ExtendedKeyMetadata], &meta) != nil || !meta.CreatedAt.Equal(record.CreatedAt) { return errVectorBucketReplaced } + tags, err := tagsFromExt(ext) + if err != nil { + return err + } principal := s.tables.PrincipalFor(r) if !s3tables.CheckPermissionWithContext(action, principal, record.OwnerAccountID, - vectorPolicyFromExt(ext), s.vectorBucketArn(bucket), s.tables.PolicyContext(r, bucket)) { + vectorPolicyFromExt(ext), s.vectorBucketArn(bucket), s.policyContext(r, bucket, tags, tags, tagRequest{})) { return errVectorDenied } return nil @@ -802,20 +935,32 @@ func (s *Server) checkVectorPolicyDoc(r *http.Request, action, bucket string, re // trusted internal path, the same pattern the implicit namespace uses. // The returned record saves the handler a second bucket lookup. func (s *Server) authorizeVector(ctx context.Context, r *http.Request, action, bucket, index string) (*s3tables.GetTableBucketResponse, error) { + return s.authorizeVectorTags(ctx, r, action, bucket, index, tagRequest{}) +} + +// authorizeVectorTags is authorizeVector for a request that sets or removes +// tags. +func (s *Server) authorizeVectorTags(ctx context.Context, r *http.Request, action, bucket, index string, req tagRequest) (*s3tables.GetTableBucketResponse, error) { record, err := s.vectorBucket(ctx, bucket) if err != nil { return nil, err } - policy, err := s.vectorPolicy(ctx, bucket) + policy, bucketTags, err := s.vectorPolicy(ctx, bucket) if err != nil { return nil, err } - resourceARN := s.vectorBucketArn(bucket) + resourceARN, resourceTags := s.vectorBucketArn(bucket), bucketTags if index != "" { - resourceARN = s.indexArn(bucket, index) + resourceARN, resourceTags = s.indexArn(bucket, index), nil + if readsResourceTags(policy) { + if resourceTags, err = s.indexTags(ctx, bucket, index); err != nil { + return nil, err + } + } } principal := s.tables.PrincipalFor(r) - if !s3tables.CheckPermissionWithContext(action, principal, record.OwnerAccountID, policy, resourceARN, s.tables.PolicyContext(r, bucket)) { + if !s3tables.CheckPermissionWithContext(action, principal, record.OwnerAccountID, policy, resourceARN, + s.policyContext(r, bucket, bucketTags, resourceTags, req)) { return nil, &s3tables.S3TablesError{Type: s3tables.ErrCodeAccessDenied, Message: "access denied"} } if !isVectorBucket(record) { @@ -888,7 +1033,7 @@ func (s *Server) handleGetVectorBucketPolicy(w http.ResponseWriter, r *http.Requ catalogErr(w, err) return } - policy, err := s.vectorPolicy(r.Context(), arns.bucket) + policy, _, err := s.vectorPolicy(r.Context(), arns.bucket) if err != nil { catalogErr(w, err) return @@ -986,13 +1131,13 @@ func (s *Server) handleCreateIndex(w http.ResponseWriter, r *http.Request) { } // The create grant is exercised before any write: a denied caller must not // leave even the implicit namespace behind. - bucketRecord, err := s.authorizeVector(r.Context(), r, "s3vectors:CreateIndex", arns.bucket, arns.index) + bucketRecord, err := s.authorizeVectorTags(r.Context(), r, "s3vectors:CreateIndex", arns.bucket, arns.index, setTags(req.Tags)) if err != nil { catalogErr(w, err) return } if len(req.Tags) > 0 { - if _, err := s.authorizeVector(r.Context(), r, "s3vectors:TagResource", arns.bucket, arns.index); err != nil { + if _, err := s.authorizeVectorTags(r.Context(), r, "s3vectors:TagResource", arns.bucket, arns.index, setTags(req.Tags)); err != nil { catalogErr(w, err) return } diff --git a/weed/s3api/s3vectors/handlers_test.go b/weed/s3api/s3vectors/handlers_test.go index 6ed043d8bc..ed1914d09e 100644 --- a/weed/s3api/s3vectors/handlers_test.go +++ b/weed/s3api/s3vectors/handlers_test.go @@ -786,7 +786,7 @@ func TestValidateVectorPolicy(t *testing.T) { if err := validateVectorPolicy("not-json"); err == nil { t.Fatal("malformed policy should be rejected") } - for _, field := range []string{"NotAction", "NotResource", "NotPrincipal", "Condition"} { + for _, field := range []string{"NotAction", "NotResource", "NotPrincipal"} { bad := fmt.Sprintf(`{"Version":"2012-10-17","Statement":[{"Effect":"Deny","Principal":"*","%s":"s3vectors:GetIndex","Action":"s3vectors:PutVectors","Resource":"*"}]}`, field) if err := validateVectorPolicy(bad); err == nil { t.Fatalf("%s should be rejected", field) diff --git a/weed/s3api/s3vectors/tag_conditions_test.go b/weed/s3api/s3vectors/tag_conditions_test.go new file mode 100644 index 0000000000..78f2786db9 --- /dev/null +++ b/weed/s3api/s3vectors/tag_conditions_test.go @@ -0,0 +1,316 @@ +package s3vectors + +import ( + "encoding/json" + "maps" + "net/http" + "testing" + + "github.com/seaweedfs/seaweedfs/weed/s3api/s3tables" +) + +// storePolicy writes a vector bucket policy straight to the bucket entry. +func (c *tagClient) storePolicy(bucket, policy string) { + c.t.Helper() + dir, name := splitEntryPath(s3tables.GetTableBucketPath(bucket)) + entry := c.fs.Get(dir, name) + if entry == nil { + c.t.Fatalf("bucket %s has no entry", bucket) + } + ext := maps.Clone(entry.Extended) + ext[vectorPolicyKey] = []byte(policy) + c.fs.Put(dir, name, ext) +} + +func (c *tagClient) setup(bucket string, indexes ...string) { + c.t.Helper() + code, body := c.op("", "CreateVectorBucket", &CreateVectorBucketRequest{VectorBucketName: bucket}) + mustStatus(c.t, "create bucket "+bucket, code, body, http.StatusOK) + for _, index := range indexes { + code, body = c.op("", "CreateIndex", &CreateIndexRequest{ + VectorBucketName: bucket, IndexName: index, DataType: "float32", Dimension: 3, + }) + mustStatus(c.t, "create index "+index, code, body, http.StatusOK) + } +} + +func (c *tagClient) getIndex(identity, bucket, index string) int { + code, _ := c.op(identity, "GetIndex", &GetIndexRequest{VectorBucketName: bucket, IndexName: index}) + return code +} + +func statement(effect, action, resource, conditions string) string { + return `{"Effect":"` + effect + `","Principal":"*","Action":` + action + `,"Resource":"` + resource + `","Condition":` + conditions + `}` +} + +func policyOf(statements ...string) string { + out := `{"Version":"2012-10-17","Statement":[` + for i, stmt := range statements { + if i > 0 { + out += "," + } + out += stmt + } + return out + `]}` +} + +func TestResourceTagReadsTheTargetsOwnTags(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket", "live-idx", "test-idx") + bucketArn := s.vectorBucketArn("vec-bucket") + mustStatus(t, "tag bucket", tagCode(c, bucketArn, map[string]string{"env": "prod"}), "", http.StatusOK) + mustStatus(t, "tag live", tagCode(c, s.indexArn("vec-bucket", "live-idx"), map[string]string{"stage": "live"}), "", http.StatusOK) + mustStatus(t, "tag test", tagCode(c, s.indexArn("vec-bucket", "test-idx"), map[string]string{"stage": "test"}), "", http.StatusOK) + c.storePolicy("vec-bucket", policyOf( + statement("Allow", `"s3vectors:GetIndex"`, bucketArn+"/index/*", `{"StringEquals":{"aws:ResourceTag/stage":"live"}}`), + statement("Allow", `"s3vectors:GetVectorBucket"`, bucketArn, `{"StringEquals":{"aws:ResourceTag/env":"prod"}}`), + )) + + if code := c.getIndex("reader", "vec-bucket", "live-idx"); code != http.StatusOK { + t.Fatalf("GetIndex on the index tagged stage=live: %d", code) + } + if code := c.getIndex("reader", "vec-bucket", "test-idx"); code != http.StatusForbidden { + t.Fatalf("GetIndex on the index tagged stage=test: %d, want 403", code) + } + getBucket := func() int { + code, _ := c.op("reader", "GetVectorBucket", &GetVectorBucketRequest{VectorBucketName: "vec-bucket"}) + return code + } + if code := getBucket(); code != http.StatusOK { + t.Fatalf("GetVectorBucket on the bucket tagged env=prod: %d", code) + } + mustStatus(t, "retag bucket", tagCode(c, bucketArn, map[string]string{"env": "dev"}), "", http.StatusOK) + if code := getBucket(); code != http.StatusForbidden { + t.Fatalf("GetVectorBucket after retagging env=dev: %d, want 403", code) + } +} + +func TestVectorBucketTagReachesIndexes(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket", "idx") + bucketArn := s.vectorBucketArn("vec-bucket") + mustStatus(t, "tag bucket", tagCode(c, bucketArn, map[string]string{"env": "prod"}), "", http.StatusOK) + mustStatus(t, "tag index", tagCode(c, s.indexArn("vec-bucket", "idx"), map[string]string{"env": "dev"}), "", http.StatusOK) + c.storePolicy("vec-bucket", policyOf( + statement("Allow", `"s3vectors:GetIndex"`, bucketArn+"/index/*", `{"StringEquals":{"s3vectors:VectorBucketTag/env":"prod"}}`), + )) + if code := c.getIndex("reader", "vec-bucket", "idx"); code != http.StatusOK { + t.Fatalf("GetIndex in a bucket tagged env=prod: %d", code) + } + mustStatus(t, "retag bucket", tagCode(c, bucketArn, map[string]string{"env": "dev"}), "", http.StatusOK) + if code := c.getIndex("reader", "vec-bucket", "idx"); code != http.StatusForbidden { + t.Fatalf("GetIndex after retagging the bucket env=dev: %d, want 403", code) + } +} + +func TestResourceTagDenyProtectsTaggedIndexes(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket", "kept", "open") + bucketArn := s.vectorBucketArn("vec-bucket") + mustStatus(t, "tag index", tagCode(c, s.indexArn("vec-bucket", "kept"), map[string]string{"protected": "true"}), "", http.StatusOK) + c.storePolicy("vec-bucket", policyOf( + `{"Effect":"Allow","Principal":"*","Action":"s3vectors:*","Resource":["`+bucketArn+`","`+bucketArn+`/index/*"]}`, + statement("Deny", `["s3vectors:DeleteIndex","s3vectors:GetIndex"]`, bucketArn+"/index/*", `{"StringEquals":{"aws:ResourceTag/protected":"true"}}`), + )) + if code := c.getIndex("reader", "vec-bucket", "kept"); code != http.StatusForbidden { + t.Fatalf("GetIndex on a protected index: %d, want 403", code) + } + if code := c.getIndex("reader", "vec-bucket", "open"); code != http.StatusOK { + t.Fatalf("GetIndex on an unprotected index: %d", code) + } + code, body := c.op("reader", "DeleteIndex", &DeleteIndexRequest{VectorBucketName: "vec-bucket", IndexName: "kept"}) + mustStatus(t, "DeleteIndex on a protected index", code, body, http.StatusForbidden) + if c.fs.Get(s3tables.GetNamespacePath("vec-bucket", indexNamespace), "kept") == nil { + t.Fatal("a refused delete removed the protected index") + } +} + +func TestRequestTagsAndTagKeysGateTagging(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket") + bucketArn := s.vectorBucketArn("vec-bucket") + mustStatus(t, "tag bucket", tagCode(c, bucketArn, map[string]string{"team": "search", "env": "dev"}), "", http.StatusOK) + c.storePolicy("vec-bucket", policyOf( + statement("Allow", `"s3vectors:TagResource"`, bucketArn, + `{"StringEquals":{"aws:RequestTag/team":"search"},"ForAllValues:StringEquals":{"aws:TagKeys":["team","env"]}}`), + statement("Allow", `"s3vectors:UntagResource"`, bucketArn, `{"ForAllValues:StringEquals":{"aws:TagKeys":["env"]}}`), + )) + for _, tc := range []struct { + tags map[string]string + want int + }{ + {map[string]string{"team": "search"}, http.StatusOK}, + {map[string]string{"team": "search", "env": "prod"}, http.StatusOK}, + {map[string]string{"team": "other"}, http.StatusForbidden}, + {map[string]string{"team": "search", "owner": "x"}, http.StatusForbidden}, + {map[string]string{"env": "prod"}, http.StatusForbidden}, + } { + code, body := c.tag("reader", bucketArn, tc.tags) + mustStatus(t, "TagResource "+jsonOf(tc.tags), code, body, tc.want) + } + code, body := c.untag("reader", bucketArn, "team") + mustStatus(t, "UntagResource team", code, body, http.StatusForbidden) + code, body = c.untag("reader", bucketArn, "env") + mustStatus(t, "UntagResource env", code, body, http.StatusOK) + if _, tags := c.list("", bucketArn); len(tags) != 1 || tags["team"] != "search" { + t.Fatalf("tags = %v, want only team=search", tags) + } +} + +func TestRequestTagGatesCreateIndex(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket") + c.storePolicy("vec-bucket", policyOf( + statement("Allow", `["s3vectors:CreateIndex","s3vectors:TagResource"]`, s.vectorBucketArn("vec-bucket")+"/index/*", + `{"StringEquals":{"aws:RequestTag/team":"search"}}`), + )) + create := func(name string, tags map[string]string) int { + code, _ := c.op("reader", "CreateIndex", &CreateIndexRequest{ + VectorBucketName: "vec-bucket", IndexName: name, DataType: "float32", Dimension: 3, Tags: tags, + }) + return code + } + if code := create("other", map[string]string{"team": "other"}); code != http.StatusForbidden { + t.Fatalf("CreateIndex tagged team=other: %d, want 403", code) + } + if code := create("untagged", nil); code != http.StatusForbidden { + t.Fatalf("CreateIndex without tags: %d, want 403", code) + } + if code := create("search", map[string]string{"team": "search"}); code != http.StatusOK { + t.Fatalf("CreateIndex tagged team=search: %d", code) + } + if code := c.getIndex("", "vec-bucket", "other"); code != http.StatusNotFound { + t.Fatalf("refused create left an index: %d", code) + } +} + +func TestResourceTagFiltersVectorBucketListing(t *testing.T) { + s, c := newTagClient(t) + for bucket, env := range map[string]string{"prod-bucket": "prod", "dev-bucket": "dev"} { + c.setup(bucket) + arn := s.vectorBucketArn(bucket) + mustStatus(t, "tag "+bucket, tagCode(c, arn, map[string]string{"env": env}), "", http.StatusOK) + c.storePolicy(bucket, policyOf( + statement("Allow", `"s3vectors:ListVectorBuckets"`, arn, `{"StringEquals":{"aws:ResourceTag/env":"prod"}}`), + )) + } + code, body := c.op("reader", "ListVectorBuckets", &ListVectorBucketsRequest{}) + mustStatus(t, "ListVectorBuckets", code, body, http.StatusOK) + var resp ListVectorBucketsResponse + if err := json.Unmarshal([]byte(body), &resp); err != nil { + t.Fatalf("decode %s: %v", body, err) + } + if len(resp.VectorBuckets) != 1 || resp.VectorBuckets[0].VectorBucketName != "prod-bucket" { + t.Fatalf("listed %s, want only prod-bucket", body) + } +} + +func TestResourceTagGatesPolicyDelete(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket") + bucketArn := s.vectorBucketArn("vec-bucket") + mustStatus(t, "tag bucket", tagCode(c, bucketArn, map[string]string{"env": "dev"}), "", http.StatusOK) + c.storePolicy("vec-bucket", policyOf( + statement("Allow", `"s3vectors:DeleteVectorBucketPolicy"`, bucketArn, `{"StringEquals":{"aws:ResourceTag/env":"prod"}}`), + )) + del := func() int { + code, _ := c.op("reader", "DeleteVectorBucketPolicy", &DeleteVectorBucketPolicyRequest{VectorBucketName: "vec-bucket"}) + return code + } + if code := del(); code != http.StatusForbidden { + t.Fatalf("DeleteVectorBucketPolicy on env=dev: %d, want 403", code) + } + mustStatus(t, "retag bucket", tagCode(c, bucketArn, map[string]string{"env": "prod"}), "", http.StatusOK) + if code := del(); code != http.StatusOK { + t.Fatalf("DeleteVectorBucketPolicy on env=prod: %d", code) + } +} + +func TestReadsResourceTags(t *testing.T) { + for policy, want := range map[string]bool{ + "": false, + `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:*","Resource":"*"}]}`: false, + `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:*","Resource":"*","Condition":{"StringEquals":{"s3vectors:VectorBucketTag/a":"b"}}}]}`: false, + `{"Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/a":"b"}}}]}`: true, + `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3vectors:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag\/a":"b"}}}]}`: true, + `{"Statement":[{"Effect":"Deny","Principal":"*","Action":"s3vectors:*","Resource":"*","Condition":{"StringEquals":{"aws:ResourceTag/a":"b"}}}]}`: true, + `not-json`: true, + } { + if got := readsResourceTags(policy); got != want { + t.Errorf("readsResourceTags(%s) = %v, want %v", policy, got, want) + } + } +} + +func tagCode(c *tagClient, arn string, tags map[string]string) int { + code, _ := c.tag("", arn, tags) + return code +} + +func jsonOf(v interface{}) string { + raw, _ := json.Marshal(v) + return string(raw) +} + +func TestValidateVectorPolicyConditions(t *testing.T) { + withCondition := func(condition string) string { + return `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","Condition":` + condition + `}]}` + } + for _, ok := range []string{ + `{"StringEquals":{"aws:ResourceTag/env":"prod"}}`, + `{"StringLike":{"aws:RequestTag/team":"search-*"}}`, + `{"ForAllValues:StringEquals":{"aws:TagKeys":["team","env"]}}`, + `{"StringNotEquals":{"s3vectors:VectorBucketTag/env":"dev"}}`, + `{"Null":{"aws:ResourceTag/owner":true}}`, + `{"StringEquals":{"aws:ResourceTag/env":["prod","staging"]}}`, + `{"NumericLessThan":{"aws:RequestTag/priority":10}}`, + } { + if err := validateVectorPolicy(withCondition(ok)); err != nil { + t.Errorf("%s rejected: %v", ok, err) + } + } + for _, bad := range []string{ + `{"StringEquals":{"aws:SourceIp":"10.0.0.1"}}`, + `{"StringEquals":{"s3tables:TableBucketTag/env":"prod"}}`, + `{"StringEquals":{"aws:ResourceTag/":"prod"}}`, + `{"StringEquals":{"aws:resourcetag/env":"prod"}}`, + `{"StringEqualsIfExists":{"aws:ResourceTag/env":"prod"}}`, + `{"NoSuchOperator":{"aws:ResourceTag/env":"prod"}}`, + `"aws:ResourceTag/env"`, + `{"StringEquals":{"aws:ResourceTag/env":null}}`, + `{"StringEquals":{"aws:ResourceTag/env":[]}}`, + `{"StringEquals":{"aws:ResourceTag/env":[null]}}`, + `{"StringEquals":{"aws:ResourceTag/env":{"a":"b"}}}`, + `{"StringEquals":{"aws:ResourceTag/env":["prod",["dev"]]}}`, + } { + if err := validateVectorPolicy(withCondition(bad)); err == nil { + t.Errorf("%s accepted", bad) + } + } + lower := `{"Version":"2012-10-17","Statement":[{"Effect":"Allow","Principal":"*","Action":"s3vectors:GetIndex","Resource":"*","condition":{"StringEquals":{"aws:ResourceTag/env":"prod"}}}]}` + if err := validateVectorPolicy(lower); err == nil { + t.Error("a lower-case condition field was accepted") + } +} + +func TestPutVectorBucketPolicyWithTagConditions(t *testing.T) { + s, c := newTagClient(t) + c.setup("vec-bucket", "live-idx", "test-idx") + mustStatus(t, "tag live", tagCode(c, s.indexArn("vec-bucket", "live-idx"), map[string]string{"stage": "live"}), "", http.StatusOK) + policy := policyOf(statement("Allow", `"s3vectors:GetIndex"`, s.vectorBucketArn("vec-bucket")+"/index/*", + `{"StringEquals":{"aws:ResourceTag/stage":"live"}}`)) + code, body := c.op("", "PutVectorBucketPolicy", &PutVectorBucketPolicyRequest{VectorBucketName: "vec-bucket", Policy: policy}) + mustStatus(t, "PutVectorBucketPolicy", code, body, http.StatusOK) + code, body = c.op("", "GetVectorBucketPolicy", &GetVectorBucketPolicyRequest{VectorBucketName: "vec-bucket"}) + mustStatus(t, "GetVectorBucketPolicy", code, body, http.StatusOK) + var got GetVectorBucketPolicyResponse + if err := json.Unmarshal([]byte(body), &got); err != nil || got.Policy != policy { + t.Fatalf("stored policy = %s (%v), want %s", body, err, policy) + } + if code := c.getIndex("reader", "vec-bucket", "live-idx"); code != http.StatusOK { + t.Fatalf("GetIndex on stage=live: %d", code) + } + if code := c.getIndex("reader", "vec-bucket", "test-idx"); code != http.StatusForbidden { + t.Fatalf("GetIndex on an untagged index: %d, want 403", code) + } +} diff --git a/weed/s3api/s3vectors/tagging.go b/weed/s3api/s3vectors/tagging.go index c7370a67f1..2b0b02d1a0 100644 --- a/weed/s3api/s3vectors/tagging.go +++ b/weed/s3api/s3vectors/tagging.go @@ -63,8 +63,8 @@ func (s *Server) tagTarget(r *http.Request, bodyArn string) (vectorBucketARNs, s // authorizeTagging grants action on the tagged bucket or index and returns a // context binding the catalog calls to the bucket it authorized. An index // target must be a vector index, not another table in its namespace. -func (s *Server) authorizeTagging(r *http.Request, action string, arns vectorBucketARNs) (context.Context, error) { - record, err := s.authorizeVector(r.Context(), r, action, arns.bucket, arns.index) +func (s *Server) authorizeTagging(r *http.Request, action string, arns vectorBucketARNs, req tagRequest) (context.Context, error) { + record, err := s.authorizeVectorTags(r.Context(), r, action, arns.bucket, arns.index, req) if err != nil { return nil, err } @@ -113,7 +113,7 @@ func (s *Server) handleTagResource(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } - ctx, err := s.authorizeTagging(r, "s3vectors:TagResource", arns) + ctx, err := s.authorizeTagging(r, "s3vectors:TagResource", arns, setTags(req.Tags)) if err != nil { catalogErr(w, err) return @@ -144,7 +144,7 @@ func (s *Server) handleUntagResource(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "ValidationException", "tagKeys are required") return } - ctx, err := s.authorizeTagging(r, "s3vectors:UntagResource", arns) + ctx, err := s.authorizeTagging(r, "s3vectors:UntagResource", arns, tagRequest{keys: req.TagKeys}) if err != nil { catalogErr(w, err) return @@ -177,7 +177,7 @@ func (s *Server) handleListTagsForResource(w http.ResponseWriter, r *http.Reques writeError(w, http.StatusBadRequest, "ValidationException", err.Error()) return } - ctx, err := s.authorizeTagging(r, "s3vectors:ListTagsForResource", arns) + ctx, err := s.authorizeTagging(r, "s3vectors:ListTagsForResource", arns, tagRequest{}) if err != nil { catalogErr(w, err) return