s3vectors: cover non-filterable key round-trip and nested filter rejection (#12160)

* s3vectors: cover non-filterable key round-trip and nested filter rejection

The catalog writes the key list as a JSON string while older records
may still carry the array form; indexFromRecord must decode both. The
rejection path extracts field names through nested $and/$or filters, so
a blocked key deep in the tree still fails the query.

* s3vectors: factor the non-filterable rejection into a testable gate

Pull the blocked-key check out of handleQueryVectors into
nonFilterableField so the rejection itself is covered without standing
up the catalog stack; the handler now 400s on whatever the gate names.

* s3vectors: report the non-filterable match separately from the key

An empty metadata key in nonFilterableMetadataKeys collided with the
"no match" sentinel, so a filter on that key skipped rejection. Return
a match flag and cover the empty-key case in the test.

weed/s3api/s3vectors/handlers.go

@@ -951,6 +951,22 @@ func translateFilter(filter map[string]interface{}) (string, error) {
 	return filterExpr(filter)
 }
 
+// nonFilterableField returns the first blocked metadata key a filter
+// references; the bool reports whether any matched.
+func nonFilterableField(cfg *MetadataConfiguration, filter map[string]interface{}) (string, bool) {
+	if cfg == nil {
+		return "", false
+	}
+	fields := map[string]bool{}
+	filterFields(filter, fields)
+	for _, k := range cfg.NonFilterableMetadataKeys {
+		if fields[k] {
+			return k, true
+		}
+	}
+	return "", false
+}
+
 // filterFields collects the field names a filter expression references —
 // $-operators recurse; every other key is a field.
 func filterFields(filter map[string]interface{}, out map[string]bool) {
@@ -1210,16 +1226,10 @@ func (s *Server) handleQueryVectors(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	idx := indexFromRecord(s, arns.bucket, arns.index, record)
-	if cfg := idx.MetadataConfiguration; cfg != nil && len(cfg.NonFilterableMetadataKeys) > 0 {
-		fields := map[string]bool{}
-		filterFields(req.Filter, fields)
-		for _, k := range cfg.NonFilterableMetadataKeys {
-			if fields[k] {
-				writeError(w, http.StatusBadRequest, "ValidationException",
-					fmt.Sprintf("metadata key %q is not filterable", k))
-				return
-			}
-		}
+	if key, blocked := nonFilterableField(idx.MetadataConfiguration, req.Filter); blocked {
+		writeError(w, http.StatusBadRequest, "ValidationException",
+			fmt.Sprintf("metadata key %q is not filterable", key))
+		return
 	}
 	metric := idx.DistanceMetric
 	filter, err := translateFilter(req.Filter)

weed/s3api/s3vectors/handlers_test.go

@@ -260,3 +260,87 @@ func TestTranslateFilter(t *testing.T) {
 		t.Fatalf("empty filter: %q %v", got, err)
 	}
 }
+
+// The catalog stores nonFilterableMetadataKeys as a JSON string; an older
+// record may still carry the array form. Both must round-trip into the index.
+func TestIndexFromRecordNonFilterableKeys(t *testing.T) {
+	s := &Server{}
+	record := func(keys interface{}) map[string]interface{} {
+		return map[string]interface{}{
+			"metadata": map[string]interface{}{
+				"fullMetadata": map[string]interface{}{
+					"dimension":                 "128",
+					"distanceMetric":            "cosine",
+					"nonFilterableMetadataKeys": keys,
+				},
+			},
+		}
+	}
+
+	idx := indexFromRecord(s, "b", "i", record(`["sku","internal"]`))
+	if idx.MetadataConfiguration == nil ||
+		len(idx.MetadataConfiguration.NonFilterableMetadataKeys) != 2 ||
+		idx.MetadataConfiguration.NonFilterableMetadataKeys[0] != "sku" ||
+		idx.MetadataConfiguration.NonFilterableMetadataKeys[1] != "internal" {
+		t.Fatalf("JSON string form: %+v", idx.MetadataConfiguration)
+	}
+
+	idx = indexFromRecord(s, "b", "i", record([]interface{}{"sku"}))
+	if idx.MetadataConfiguration == nil ||
+		len(idx.MetadataConfiguration.NonFilterableMetadataKeys) != 1 ||
+		idx.MetadataConfiguration.NonFilterableMetadataKeys[0] != "sku" {
+		t.Fatalf("array form: %+v", idx.MetadataConfiguration)
+	}
+
+	for _, absent := range []interface{}{nil, "not-json", `{"a":1}`} {
+		idx = indexFromRecord(s, "b", "i", record(absent))
+		if idx.MetadataConfiguration != nil {
+			t.Fatalf("keys %v should leave configuration unset, got %+v", absent, idx.MetadataConfiguration)
+		}
+	}
+}
+
+// QueryVectors rejects filters touching non-filterable keys; the extraction
+// must see through nested boolean operators.
+func TestFilterFieldsNestedOperators(t *testing.T) {
+	filter := map[string]interface{}{
+		"$and": []interface{}{
+			map[string]interface{}{"color": "red"},
+			map[string]interface{}{"$or": []interface{}{
+				map[string]interface{}{"sku": map[string]interface{}{"$eq": "x"}},
+				map[string]interface{}{"$and": []interface{}{
+					map[string]interface{}{"size": map[string]interface{}{"$gt": 3}},
+				}},
+			}},
+		},
+	}
+	fields := map[string]bool{}
+	filterFields(filter, fields)
+	for _, want := range []string{"color", "sku", "size"} {
+		if !fields[want] {
+			t.Fatalf("missing field %q in %v", want, fields)
+		}
+	}
+	if fields["$and"] || fields["$or"] || fields["$eq"] {
+		t.Fatalf("operators leaked into fields: %v", fields)
+	}
+
+	// The rejection gate the handler applies to the extracted fields.
+	blocked := &MetadataConfiguration{NonFilterableMetadataKeys: []string{"sku"}}
+	if got, hit := nonFilterableField(blocked, filter); !hit || got != "sku" {
+		t.Fatalf("blocked key inside nested filter not detected: %q %v", got, hit)
+	}
+	allowed := &MetadataConfiguration{NonFilterableMetadataKeys: []string{"secret"}}
+	if got, hit := nonFilterableField(allowed, filter); hit {
+		t.Fatalf("unrelated non-filterable key should pass, got %q", got)
+	}
+	if got, hit := nonFilterableField(nil, filter); hit {
+		t.Fatalf("no configuration should pass, got %q", got)
+	}
+	emptyBlocked := &MetadataConfiguration{NonFilterableMetadataKeys: []string{""}}
+	if _, hit := nonFilterableField(emptyBlocked, map[string]interface{}{
+		"": map[string]interface{}{"$eq": "x"},
+	}); !hit {
+		t.Fatal("an empty blocked key should still reject")
+	}
+}