fix: let a connection open when CLIENT SETINFO is denied

docs/user_guide/installation.md

@@ -211,7 +211,9 @@ Index enumeration is the only thing an `-@admin` rule breaks. It is reached by `
 
 Other categories gate different operations. `FT.DROPINDEX` is tagged `@dangerous` and `@write` as well as `@search`, so a policy that subtracts `@dangerous` denies `index.delete()`, `rvl index delete`, and `rvl index destroy`.
 
-Outside of Redis Search, RedisVL identifies itself on connect with `CLIENT SETINFO` and falls back to `ECHO`. A credential permitted to run neither currently fails when the connection is created, before any index operation.
+Outside of Redis Search, RedisVL identifies itself on connect with `CLIENT SETINFO`. That command is tagged `@connection` and `@slow`, and belongs to neither `@read` nor `@write`, so a rule built up from those categories never grants it. A credential that cannot run it still connects: identification only populates the `lib-name` field that `CLIENT LIST` and `CLIENT INFO` display, so a refusal is ignored (and logged, if you have configured logging at debug level). Grant `+client|setinfo` if you want RedisVL to appear as the connecting library there — note that this labels the connection RedisVL opens, while redis-py labels the rest of the pool as plain `redis-py`.
+
+Cluster deployments need one more grant. `RedisCluster` discovers the topology with `CLUSTER SLOTS`, which is tagged `@slow` only, so a credential assembled from `+@read +@write` cannot open a clustered connection at all — redis-py reports this as `Redis Cluster cannot be connected`, with the underlying permission error chained beneath it. Grant `+cluster|slots` alongside the rules above.
 
 ### Redis Cloud and Redis Software
 

redisvl/redis/connection.py

@@ -212,6 +212,43 @@ def make_lib_name(*args) -> str:
     return f"redis-py({custom_libs})"
 
 
+def _identify_client(client: SyncRedisClient, lib_name: str | None = None) -> None:
+    """Report RedisVL as the connecting library, tolerating a refusal.
+
+    redis-py sends its own ``CLIENT SETINFO`` during the connection handshake,
+    so this call is here to overwrite that with the composed
+    ``redis-py(redisvl_v...;<wrapper>)`` name that adoption metrics read. Keep
+    it: without it the library and any wrapper above it go unattributed.
+
+    A refusal is ignored, because ``CLIENT SETINFO`` only populates the
+    ``lib-name`` field that ``CLIENT LIST`` and ``CLIENT INFO`` display -- its
+    own documentation tells client libraries to ignore failures. Two credentials
+    hit this: one granting neither ``@connection`` nor the command itself, and
+    one on a server predating Redis 7.2, where the command does not exist.
+
+    Only ``ResponseError`` is caught. In the connection-factory path this is the
+    first command issued on a freshly created connection, which makes it the
+    de-facto connectivity check, so swallowing ``ConnectionError`` would defer a
+    genuine failure to some later and more confusing command. Note that on a
+    cluster client redis-py routes the command to the default node only, so the
+    label reaches one node rather than the whole cluster.
+    """
+    try:
+        client.client_setinfo("LIB-NAME", make_lib_name(lib_name))
+    except ResponseError as e:
+        logger.debug(f"CLIENT SETINFO was not applied, continuing without it: {e}")
+
+
+async def _aidentify_client(
+    client: AsyncRedisClient, lib_name: str | None = None
+) -> None:
+    """Async version of :func:`_identify_client`."""
+    try:
+        await client.client_setinfo("LIB-NAME", make_lib_name(lib_name))
+    except ResponseError as e:
+        logger.debug(f"CLIENT SETINFO was not applied, continuing without it: {e}")
+
+
 def convert_index_info_to_schema(index_info: dict[str, Any]) -> dict[str, Any]:
     """Convert the output of FT.INFO into a schema-ready dictionary.
 
@@ -540,15 +577,7 @@ def get_redis_connection(
             client = RedisCluster.from_url(url, **kwargs)
         else:
             client = Redis.from_url(url, **kwargs)
-        # Module validation removed - operations will fail naturally if modules are missing
-        # Set client library name only
-        _lib_name = make_lib_name(kwargs.get("lib_name"))
-        try:
-            client.client_setinfo("LIB-NAME", _lib_name)
-        except ResponseError:
-            # Fall back to a simple log echo
-            if hasattr(client, "echo"):
-                client.echo(_lib_name)
+        _identify_client(client, kwargs.get("lib_name"))
         return client
 
     @staticmethod
@@ -599,15 +628,7 @@ async def _get_aredis_connection(
             )
             client = AsyncRedis.from_url(cleaned_url, **cleaned_kwargs)
 
-        # Module validation removed - operations will fail naturally if modules are missing
-        # Set client library name only
-        _lib_name = make_lib_name(kwargs.get("lib_name"))
-        try:
-            await client.client_setinfo("LIB-NAME", _lib_name)
-        except ResponseError:
-            # Fall back to a simple log echo
-            if hasattr(client, "echo"):
-                await client.echo(_lib_name)
+        await _aidentify_client(client, kwargs.get("lib_name"))
         return client
 
     @staticmethod
@@ -718,52 +739,50 @@ def validate_sync_redis(
         redis_client: SyncRedisClient,
         lib_name: str | None = None,
     ) -> None:
-        """Validates the sync Redis client.
+        """Check the client type and report the library name.
 
-        Note: Module validation has been removed. This method now only validates
-        the client type and sets the library name.
+        Identification is best effort: a server that refuses ``CLIENT SETINFO``
+        is tolerated, so the only failure raised here is a wrong client type.
+        (Module validation was removed; a missing module now surfaces when an
+        operation needs it.)
+
+        Args:
+            redis_client (SyncRedisClient): The client to check.
+            lib_name (Optional[str]): Name of a library wrapping RedisVL, to
+                report alongside it. Defaults to None.
+
+        Raises:
+            TypeError: If the client is not a Redis or RedisCluster instance.
         """
         if not issubclass(type(redis_client), (Redis, RedisCluster)):
             raise TypeError(
                 "Invalid Redis client instance. Must be Redis or RedisCluster."
             )
 
-        # Set client library name
-        _lib_name = make_lib_name(lib_name)
-        try:
-            redis_client.client_setinfo("LIB-NAME", _lib_name)
-        except ResponseError:
-            # Fall back to a simple log echo
-            # For RedisCluster, echo is not available
-            if hasattr(redis_client, "echo"):
-                redis_client.echo(_lib_name)
-
-        # Module validation removed - operations will fail naturally if modules are missing
+        _identify_client(redis_client, lib_name)
 
     @staticmethod
     async def validate_async_redis(
         redis_client: AsyncRedisClient,
         lib_name: str | None = None,
     ) -> None:
-        """Validates the async Redis client.
+        """Async version of :meth:`validate_sync_redis`.
 
-        Note: Module validation has been removed. This method now only validates
-        the client type and sets the library name.
+        Args:
+            redis_client (AsyncRedisClient): The client to check.
+            lib_name (Optional[str]): Name of a library wrapping RedisVL, to
+                report alongside it. Defaults to None.
+
+        Raises:
+            TypeError: If the client is not an async Redis or RedisCluster
+                instance.
         """
         if not issubclass(type(redis_client), (AsyncRedis, AsyncRedisCluster)):
             raise TypeError(
                 "Invalid async Redis client instance. Must be async Redis or async RedisCluster."
             )
-        # Set client library name
-        _lib_name = make_lib_name(lib_name)
-        try:
-            await redis_client.client_setinfo("LIB-NAME", _lib_name)
-        except ResponseError:
-            # Fall back to a simple log echo
-            if hasattr(redis_client, "echo"):
-                await redis_client.echo(_lib_name)
 
-        # Module validation removed - operations will fail naturally if modules are missing
+        await _aidentify_client(redis_client, lib_name)
 
     @staticmethod
     @overload

tests/conftest.py

@@ -4,14 +4,17 @@
 import re
 import subprocess
 import sys
+from contextlib import contextmanager, suppress
 from datetime import datetime, timezone
 
 import pytest
+from redis.exceptions import ResponseError
 from testcontainers.compose import DockerCompose
 
 from redisvl.index.index import AsyncSearchIndex, SearchIndex
 from redisvl.redis.connection import RedisConnectionFactory, is_version_gte
 from redisvl.redis.utils import array_to_buffer
+from redisvl.types import SyncRedisClient
 
 # Check if we're on Python 3.14+ where sentence-transformers may not work
 SKIP_HF = sys.version_info >= (3, 14)
@@ -212,6 +215,83 @@ def client(redis_url):
     yield conn
 
 
+class _AclUser:
+    """A temporary ACL user, plus the RedisVL connections opened as them.
+
+    `username` and `password` are public so a test can hand them to anything
+    that takes connection kwargs, such as an extension constructor, instead of
+    going through `connect()`.
+    """
+
+    def __init__(self, username: str, password: str, redis_url: str):
+        self.username = username
+        self.password = password
+        self._redis_url = redis_url
+        self._clients: list[SyncRedisClient] = []
+
+    def connect(self, **kwargs):
+        """Open a RedisVL connection authenticated as this user."""
+        conn = RedisConnectionFactory.get_redis_connection(
+            redis_url=self._redis_url,
+            username=self.username,
+            password=self.password,
+            **kwargs,
+        )
+        self._clients.append(conn)
+        return conn
+
+    def close_all(self) -> None:
+        for conn in self._clients:
+            with suppress(Exception):
+                conn.close()
+
+
+@pytest.fixture
+def acl_user(client, redis_url, redis_test_name):
+    """Create a temporary ACL user, yielding a handle that connects as them.
+
+    The returned factory takes a required `name` and the ACL rules to apply, in
+    the order Redis applies them: rules are evaluated left to right, so
+    `+@all -@admin` and `-@admin +@all` do not mean the same thing. `on` and a
+    password are supplied here, so callers pass only key patterns, channel
+    patterns and command rules::
+
+        with acl_user("~*", "&*", "+@read", "+@write", name="read_write") as user:
+            restricted = user.connect()
+
+    Rules are applied after `reset`, because `ACL SETUSER` is additive and
+    usernames are derived from the test's node id -- without it, a run whose
+    teardown was skipped by a crash would layer new rules onto a stale user.
+
+    ACL users are server-global, so a leaked one outlives the test and its
+    worker; the user is dropped before the connections it authenticated. Redis
+    Software manages ACLs through its own control plane and does not support the
+    `>password` syntax used here, so tests built on this fixture only run
+    against Redis Open Source -- they skip elsewhere.
+    """
+
+    @contextmanager
+    def _make_acl_user(*rules: str, name: str):
+        username = redis_test_name(name)
+        password = "test-acl-password"
+        try:
+            client.execute_command(
+                "ACL", "SETUSER", username, "reset", "on", f">{password}", *rules
+            )
+        except ResponseError as e:
+            pytest.skip(f"Deployment does not support ACL SETUSER: {e}")
+        user = _AclUser(username, password, redis_url)
+        try:
+            yield user
+        finally:
+            try:
+                client.execute_command("ACL", "DELUSER", username)
+            finally:
+                user.close_all()
+
+    return _make_acl_user
+
+
 @pytest.fixture
 def cluster_client(redis_cluster_url):
     """

tests/integration/test_connection.py

@@ -3,7 +3,7 @@
 import pytest
 from redis import Redis
 from redis.asyncio import Redis as AsyncRedis
-from redis.exceptions import ConnectionError
+from redis.exceptions import ConnectionError, NoPermissionError
 
 from redisvl.redis.connection import (
     RedisConnectionFactory,
@@ -136,6 +136,31 @@ def test_unknown_redis(self):
             bad_client.ping()
 
 
+def test_connection_opens_without_identification_permission(acl_user):
+    """A credential that cannot identify the client must still connect.
+
+    RedisVL announces itself with CLIENT SETINFO when a connection is made, and
+    used to fall back to ECHO if that was refused. Both commands are
+    `@connection` and are in neither `@read` nor `@write`, so a role built up
+    from those categories never grants either -- and the unguarded fallback made
+    RedisVL fail while the connection was being created, before any index
+    operation could be attempted.
+
+    Only a live server can show that this role really denies the command, which
+    is why this is an integration test.
+    """
+    with acl_user("~*", "&*", "+@read", "+@write", name="acl_no_connection") as user:
+        restricted = user.connect()
+
+        # Pin the premise. If a future Redis lets this role run CLIENT SETINFO,
+        # the test is no longer exercising the tolerance and should say so.
+        with pytest.raises(NoPermissionError):
+            restricted.client_setinfo("LIB-NAME", "probe")
+
+        # And the connection is genuinely usable for what the role does permit.
+        assert restricted.exists("no-such-key") == 0
+
+
 def test_validate_redis(client):
     skip_if_redis_version_below(client, "7.2.0")
     RedisConnectionFactory.validate_sync_redis(client)

tests/integration/test_search_index.py

@@ -313,9 +313,7 @@ def test_search_index_delete(index):
     assert index.name not in index.listall()
 
 
-def test_exists_under_acl_without_admin_commands(
-    index, client, redis_url, redis_test_name
-):
+def test_exists_under_acl_without_admin_commands(index, client, acl_user):
     """exists() must work for a credential that grants search but drops @admin.
 
     Redis tags FT._LIST @admin as well as @search, so this ACL shape used to
@@ -327,30 +325,18 @@ def test_exists_under_acl_without_admin_commands(
     skip_if_no_redis_search(client)
     index.create(overwrite=True, drop=True)
 
-    username = redis_test_name("acl_no_admin")
-    password = "test-acl-password"
-    client.execute_command(
-        "ACL", "SETUSER", username, "on", f">{password}", "~*", "&*", "+@all", "-@admin"
-    )
-    restricted = None
     try:
-        restricted = RedisConnectionFactory.get_redis_connection(
-            redis_url=redis_url, username=username, password=password
-        )
-        # Pin the premise so this test cannot quietly become vacuous: if a
-        # future Redis stops gating FT._LIST behind @admin, the reason for
-        # preferring FT.INFO is gone and we want to hear about it here.
-        with pytest.raises(NoPermissionError):
-            restricted.execute_command("FT._LIST")
-
-        restricted_index = SearchIndex(schema=index.schema, redis_client=restricted)
-        assert restricted_index.exists() is True
+        with acl_user("~*", "&*", "+@all", "-@admin", name="acl_no_admin") as user:
+            restricted = user.connect()
+            # Pin the premise so this test cannot quietly become vacuous: if a
+            # future Redis stops gating FT._LIST behind @admin, the reason for
+            # preferring FT.INFO is gone and we want to hear about it here.
+            with pytest.raises(NoPermissionError):
+                restricted.execute_command("FT._LIST")
+
+            restricted_index = SearchIndex(schema=index.schema, redis_client=restricted)
+            assert restricted_index.exists() is True
     finally:
-        # Drop the ACL user before anything else that could raise: users are
-        # server-global, so a leaked one outlives this test and its worker.
-        client.execute_command("ACL", "DELUSER", username)
-        if restricted is not None:
-            restricted.close()
         index.delete(drop=True)