fix(mcp): escape text filter values and reject an empty projection

redisvl/mcp/config.py

@@ -472,10 +472,22 @@ def validate_runtime_mapping(self, schema: IndexSchema) -> None:
         """Ensure runtime mappings point at explicit fields in the effective schema."""
         field_names = set(schema.field_names)
 
-        if self.uses_text_search and self.runtime.text_field_name not in field_names:
-            raise ValueError(
-                f"runtime.text_field_name '{self.runtime.text_field_name}' not found in schema"
-            )
+        if self.uses_text_search:
+            if self.runtime.text_field_name not in field_names:
+                raise ValueError(
+                    f"runtime.text_field_name '{self.runtime.text_field_name}' not found in schema"
+                )
+            # Membership alone is not enough: a vector field cannot be searched as
+            # text, and pointing at one leaves the default projection empty (it is
+            # every *non-vector* field). An empty projection reaches Redis as no
+            # RETURN clause at all, which returns every stored field -- including
+            # the embedding this tool refuses to return when asked for by name.
+            text_field = schema.fields.get(self.runtime.text_field_name)
+            if text_field is not None and text_field.type == "vector":
+                raise ValueError(
+                    f"runtime.text_field_name '{self.runtime.text_field_name}' is a "
+                    "vector field; text search requires a text or tag field"
+                )
 
         if (
             self.supports_server_side_embedding

redisvl/mcp/filters.py

@@ -1,8 +1,32 @@
+import re
 from typing import Any, Iterable
 
 from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError
 from redisvl.query.filter import FilterExpression, Num, Tag, Text
 from redisvl.schema import IndexSchema
+from redisvl.utils.token_escaper import TokenEscaper
+
+# Text values reach the query string unescaped: `Text`'s operator templates are
+# `@field:("value")` for eq/ne and `@field:(value)` for like, so a caller value
+# containing a quote or a paren can close its own clause and inject arbitrary
+# RediSearch syntax after it -- including a `|` that escapes an enclosing AND.
+# Tag and numeric values are already escaped or type-checked upstream; text is
+# not, so this boundary escapes it before building the expression.
+_TEXT_ESCAPER = TokenEscaper()
+
+# `like` is the pattern operator, so the metacharacters that give a pattern its
+# meaning have to stay live: `*` and `?` for wildcards, `%` for fuzzy matching,
+# and a space for the implicit AND between terms. Escaping those does not fail
+# loudly -- it silently turns a documented pattern into a literal that matches
+# nothing -- so this set is the shared no-wildcard set minus `%` and space.
+#
+# Dropping them costs nothing in containment, because containment comes from the
+# delimiters rather than from these. With `(` and `)` escaped, the value cannot
+# close its own `@field:(...)`, so anything it carries -- including a `|`, which
+# no escaper in RedisVL touches -- stays scoped to this one field instead of
+# reaching the surrounding expression.
+_LIKE_ESCAPED_CHARS = re.compile(r"[,.<>{}\[\]\\\"\':;!@#$^&()\-+=~\/]")
+_LIKE_ESCAPER = TokenEscaper(escape_chars_re=_LIKE_ESCAPED_CHARS)
 
 
 def parse_filter(
@@ -132,17 +156,32 @@ def _parse_tag_expression(field_name: str, op: str, operand: Any) -> FilterExpre
     )
 
 
+def _escape_text(value: str) -> str:
+    """Escape a caller-supplied text value so it cannot leave its own clause."""
+    return _TEXT_ESCAPER.escape(value)
+
+
+def _escape_like_pattern(value: str) -> str:
+    """Escape a `like` pattern, leaving its pattern metacharacters intact."""
+    return _LIKE_ESCAPER.escape(value)
+
+
 def _parse_text_expression(field_name: str, op: str, operand: Any) -> FilterExpression:
     field = Text(field_name)
     if op == "eq":
-        return field == _require_string(operand, field_name, op)
+        return field == _escape_text(_require_string(operand, field_name, op))
     if op == "ne":
-        return field != _require_string(operand, field_name, op)
+        return field != _escape_text(_require_string(operand, field_name, op))
     if op == "like":
-        return field % _require_string(operand, field_name, op)
+        # An exact-match value is a literal, but a `like` value is a pattern, so
+        # the two need different escaping -- see `_LIKE_ESCAPED_CHARS`.
+        return field % _escape_like_pattern(_require_string(operand, field_name, op))
     if op == "in":
         return _combine_or(
-            [field == item for item in _require_string_list(operand, field_name, op)]
+            [
+                field == _escape_text(item)
+                for item in _require_string_list(operand, field_name, op)
+            ]
         )
     raise RedisVLMCPError(
         f"Unsupported operator '{op}' for text field '{field_name}'",

redisvl/mcp/tools/search.py

@@ -153,6 +153,17 @@ def _validate_request(
                 code=MCPErrorCode.INVALID_REQUEST,
                 retryable=False,
             )
+        if not return_fields:
+            # An empty projection reaches Redis as no RETURN clause at all, which
+            # returns *every* field -- including the vector this function refuses
+            # a few lines down. "Omit the argument" is the way to ask for the
+            # default projection.
+            raise RedisVLMCPError(
+                "return_fields must not be empty; omit it to use the default "
+                "projection",
+                code=MCPErrorCode.INVALID_REQUEST,
+                retryable=False,
+            )
         fields = []
         for field_name in return_fields:
             if not isinstance(field_name, str) or not field_name:

tests/unit/test_mcp/conftest.py

@@ -1,8 +1,43 @@
 import pytest
 
+from redisvl.schema import IndexSchema
+
 
 @pytest.fixture(scope="session", autouse=True)
 def redis_container():
     # Shadow the repo-wide autouse Redis container fixture so MCP unit tests stay
     # pure-unit and do not require Docker; Redis coverage lives in integration tests.
     yield None
+
+
+def _schema() -> IndexSchema:
+    """The one index shape the filter, search, and profile unit tests all build against.
+
+    A plain function rather than a fixture: it is a stateless data builder that
+    module-level parametrization and helper functions both need to call, and the
+    convention in these files is per-module fakes rather than shared fixtures.
+    """
+    return IndexSchema.from_dict(
+        {
+            "index": {
+                "name": "docs-index",
+                "prefix": "doc",
+                "storage_type": "hash",
+            },
+            "fields": [
+                {"name": "content", "type": "text"},
+                {"name": "category", "type": "tag"},
+                {"name": "rating", "type": "numeric"},
+                {
+                    "name": "embedding",
+                    "type": "vector",
+                    "attrs": {
+                        "algorithm": "flat",
+                        "dims": 3,
+                        "distance_metric": "cosine",
+                        "datatype": "float32",
+                    },
+                },
+            ],
+        }
+    )

tests/unit/test_mcp/test_config.py

@@ -525,3 +525,36 @@ def test_mcp_config_allows_linear_hybrid_fallback_params():
         schema=schema,
         supports_native_hybrid_search=False,
     )
+
+
+@pytest.mark.parametrize("search_type", ["fulltext", "hybrid"])
+def test_mcp_config_rejects_a_text_field_that_is_actually_the_vector_field(search_type):
+    """Membership in the schema is not enough -- the field must be text-searchable.
+
+    Pointing `text_field_name` at the vector field passes a name-only check while
+    leaving the default projection (every *non-vector* field) empty. An empty
+    projection reaches Redis as no RETURN clause, which returns every stored
+    field including the embedding, so this has to fail at startup.
+    """
+    config = _valid_config()
+    config["indexes"]["knowledge"]["search"] = {"type": search_type}
+    config["indexes"]["knowledge"]["runtime"]["text_field_name"] = "embedding"
+
+    binding = MCPConfig.model_validate(config).indexes["knowledge"]
+
+    # `to_index_schema` validates the runtime mapping against the effective
+    # schema, so startup fails here rather than at the first request.
+    with pytest.raises(ValueError, match="is a vector field"):
+        binding.to_index_schema(_inspected_schema())
+
+
+def test_mcp_config_still_accepts_a_real_text_field():
+    """The control: a genuine text field must keep validating."""
+    config = _valid_config()
+    config["indexes"]["knowledge"]["search"] = {"type": "fulltext"}
+    config["indexes"]["knowledge"]["runtime"]["text_field_name"] = "content"
+
+    binding = MCPConfig.model_validate(config).indexes["knowledge"]
+    schema = binding.to_index_schema(_inspected_schema())
+
+    binding.validate_runtime_mapping(schema)

tests/unit/test_mcp/test_filters.py

@@ -1,36 +1,9 @@
 import pytest
+from conftest import _schema
 
 from redisvl.mcp.errors import MCPErrorCode, RedisVLMCPError
 from redisvl.mcp.filters import parse_filter
-from redisvl.query.filter import FilterExpression
-from redisvl.schema import IndexSchema
-
-
-def _schema() -> IndexSchema:
-    return IndexSchema.from_dict(
-        {
-            "index": {
-                "name": "docs-index",
-                "prefix": "doc",
-                "storage_type": "hash",
-            },
-            "fields": [
-                {"name": "content", "type": "text"},
-                {"name": "category", "type": "tag"},
-                {"name": "rating", "type": "numeric"},
-                {
-                    "name": "embedding",
-                    "type": "vector",
-                    "attrs": {
-                        "algorithm": "flat",
-                        "dims": 3,
-                        "distance_metric": "cosine",
-                        "datatype": "float32",
-                    },
-                },
-            ],
-        }
-    )
+from redisvl.query.filter import FilterExpression, Text
 
 
 def _render_filter(value):
@@ -39,6 +12,18 @@ def _render_filter(value):
     return value
 
 
+def _strip_escapes(rendered: str) -> str:
+    """Drop backslash-escaped pairs, leaving only unescaped query syntax."""
+    out, index = [], 0
+    while index < len(rendered):
+        if rendered[index] == "\\":
+            index += 2
+            continue
+        out.append(rendered[index])
+        index += 1
+    return "".join(out)
+
+
 def test_parse_filter_passes_through_raw_string():
     raw = "@category:{science} @rating:[4 +inf]"
 
@@ -134,3 +119,85 @@ def test_parse_filter_rejects_malformed_payload():
         parse_filter({"field": "category", "value": "science"}, _schema())
 
     assert exc_info.value.code == MCPErrorCode.INVALID_FILTER
+
+
+@pytest.mark.parametrize(
+    ("op", "operand"),
+    [
+        ("eq", 'alpha") | (-@category:{secret}'),
+        ("ne", 'alpha") | (-@category:{secret}'),
+        ("like", "alpha) | (-@category:{secret}"),
+        ("in", ['alpha") | (-@category:{secret}']),
+    ],
+)
+def test_parse_filter_escapes_text_values_so_they_cannot_leave_their_clause(
+    op, operand
+):
+    parsed = parse_filter({"field": "content", "op": op, "value": operand}, _schema())
+
+    # Text operator templates interpolate the value into `@field:("...")` or
+    # `@field:(...)`, so an unescaped quote or paren would close the clause and
+    # let the rest of the value inject query syntax -- including a `|` that
+    # escapes an enclosing AND. Stripping escape pairs leaves the structural
+    # skeleton: the payload must contribute no syntax to it.
+    skeleton = _strip_escapes(_render_filter(parsed))
+    # The clause boundary survives: quotes and parens from the payload are
+    # escaped, so its `|` stays scoped inside this field's own query instead of
+    # splitting the whole expression into a union.
+    assert skeleton.count("(") == skeleton.count(")")
+    assert skeleton.count('"') % 2 == 0
+
+
+@pytest.mark.parametrize(
+    ("value", "rendered"),
+    [
+        # Each metacharacter that gives a `like` pattern its meaning. Escaping any
+        # of these does not fail loudly -- it turns the pattern into a literal that
+        # matches nothing -- so each is pinned separately.
+        ("quant*", "@content:(quant*)"),
+        ("qu?nt", "@content:(qu?nt)"),
+        # A space is the implicit AND between terms, not padding.
+        ("foo bar", "@content:(foo bar)"),
+        # `%` is fuzzy matching.
+        ("%foo%", "@content:(%foo%)"),
+    ],
+)
+def test_parse_filter_preserves_like_pattern_metacharacters(value, rendered):
+    parsed = parse_filter({"field": "content", "op": "like", "value": value}, _schema())
+
+    assert _render_filter(parsed) == rendered
+
+
+def test_parse_filter_like_matches_library_semantics_for_patterns():
+    """MCP `like` must not diverge from `Text.__mod__` for ordinary patterns."""
+    for value in ["quant*", "foo bar", "%foo%", "qu?nt"]:
+        mcp = _render_filter(
+            parse_filter({"field": "content", "op": "like", "value": value}, _schema())
+        )
+        assert mcp == str(Text("content") % value), f"diverged for {value!r}"
+
+
+def test_parse_filter_like_still_escapes_clause_delimiters():
+    """Preserving pattern metacharacters must not cost containment.
+
+    Containment comes from the delimiters, not from `%`/space: with `(` and `)`
+    escaped the value cannot close its own `@field:(...)`, so a `|` it carries
+    stays scoped to this field. `|` is deliberately checked because no escaper in
+    RedisVL touches it.
+    """
+    parsed = parse_filter(
+        {"field": "content", "op": "like", "value": "alpha) | (-@category:{secret}"},
+        _schema(),
+    )
+    rendered = _render_filter(parsed)
+
+    # The payload's own parens are escaped...
+    assert "\\)" in rendered and "\\(" in rendered
+    # ...so stripping escape pairs leaves a balanced skeleton: nothing the payload
+    # contributed can terminate the clause early.
+    skeleton = _strip_escapes(rendered)
+    assert skeleton.count("(") == skeleton.count(")")
+    # The `|` survives unescaped but is inside the field clause, so it unions
+    # within `content` rather than splitting the whole expression.
+    assert skeleton.index("|") > skeleton.index("(")
+    assert skeleton.index("|") < skeleton.rindex(")")