Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions docs/user_guide/installation.md
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,18 @@ RedisSearchError: Error while fetching llmcache index info:
User <name> has no permissions to run the 'FT.INFO' command
```

Reading a cache is not read-only either. When a cache has a TTL configured — at construction or later via `set_ttl()` — every read that hits refreshes the matched entries' TTL, so it issues `EXPIRE`. That includes `SemanticCache.check()`, every `EmbeddingsCache` getter, their async equivalents, and the reads a vectorizer performs for you when constructed with `cache=`. `EXPIRE` is in `@write`, not `@read` (measured with `ACL CAT write`), so a lookup-only credential fails on a cache *hit* rather than on the write that populated it.

Granting the command is only half of it, because the key patterns must permit writes too. Measured on 8.4.5 with `ACL DRYRUN <user> EXPIRE llmcache:abc 60`:

| Rules | Result |
|---|---|
| `+@read ~llmcache:*` | `no permissions to run the 'expire' command` |
| `+@read +expire ~llmcache:*` | Permitted |
| `+@read +expire %R~llmcache:*` | `no permissions to access the 'llmcache:abc' key` |

So `%R~llmcache:*` — the read-only shape the [Key permissions](#key-permissions) table below presents as sufficient for querying — is denied on the key, not the command. `+@read +@write` is unaffected, as is a cache with no TTL. See [Cache LLM Responses](03_llmcache.ipynb) for the refresh behaviour itself, including that `set_ttl()` starts adding TTLs to entries stored without one.

### "no permissions to run the 'FT.INFO' command"

RedisVL does not guess its way around this. A credential that cannot ask whether the index exists also cannot create one, so there is nothing useful to infer — instead, tell RedisVL that the index is already there:
Expand Down Expand Up @@ -281,6 +293,8 @@ Measured on 8.4.5 against an index prefixed `doc:`, with the command categories

Partial overlap is worth emphasising: it fails exactly like no overlap at all, rather than returning the subset you can read. `FT.CREATE` is not checked this way, so a credential can create an index it is then unable to query.

Key patterns are glob-style, matched by the same engine as `SCAN MATCH`, so metacharacters in a prefix do not mean what they look like. Measured on 8.4.5, `~cache[ab]:*` grants `cachea:1` and `cacheb:1` while *denying* the literal key `cache[ab]:1`; escaped as `~cache\[ab]:*` it does the reverse. Escape `*`, `?`, `[` and `\` in a prefix, and confirm the rule with `ACL DRYRUN`.

`create_index=False` does not help here — the very commands it lets you avoid are joined by the ones it cannot, so widen the key patterns instead.

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`.
Expand Down
9 changes: 5 additions & 4 deletions redisvl/extensions/cache/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

from redisvl.redis.connection import RedisConnectionFactory
from redisvl.types import AsyncRedisClient, SyncRedisClient
from redisvl.utils.utils import match_pattern


class BaseCache:
Expand Down Expand Up @@ -184,12 +185,12 @@ async def aexpire(self, key: str, ttl: int | None = None) -> None:
def clear(self) -> None:
"""Clear the cache of all keys."""
client = self._get_redis_client()
prefix = self._get_prefix()
pattern = match_pattern(self._get_prefix())

# Scan for all keys with our prefix
cursor = 0 # Start with cursor 0
while True:
cursor_int, keys = client.scan(cursor=cursor, match=f"{prefix}*", count=100) # type: ignore
cursor_int, keys = client.scan(cursor=cursor, match=pattern, count=100) # type: ignore
if keys:
client.delete(*keys)
if cursor_int == 0: # Redis returns 0 when scan is complete
Expand All @@ -206,13 +207,13 @@ def clear(self) -> None:
async def aclear(self) -> None:
"""Async clear the cache of all keys."""
client = await self._get_async_redis_client()
prefix = self._get_prefix()
pattern = match_pattern(self._get_prefix())

# Scan for all keys with our prefix
cursor = 0 # Start with cursor 0
while True:
cursor_int, keys = await client.scan(
cursor=cursor, match=f"{prefix}*", count=100
cursor=cursor, match=pattern, count=100
) # type: ignore
if keys:
await client.delete(*keys)
Expand Down
11 changes: 8 additions & 3 deletions redisvl/extensions/router/semantic.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,12 @@
from redisvl.redis.utils import convert_bytes, hashify, make_dict
from redisvl.types import SyncRedisClient
from redisvl.utils.log import get_logger
from redisvl.utils.utils import deprecated_argument, model_to_dict, scan_by_pattern
from redisvl.utils.utils import (
deprecated_argument,
match_pattern,
model_to_dict,
scan_by_pattern,
)
from redisvl.utils.vectorize.base import BaseVectorizer
from redisvl.utils.vectorize.text.huggingface import HFTextVectorizer

Expand Down Expand Up @@ -336,9 +341,9 @@ def _route_pattern(index: SearchIndex, route_name: str) -> str:
# Normalize prefix to avoid double separators
prefix = index.prefix.rstrip(sep) if sep and index.prefix else index.prefix
if prefix:
return f"{prefix}{sep}{route_name}{sep}*"
return match_pattern(prefix, sep, route_name, sep)
else:
return f"{route_name}{sep}*"
return match_pattern(route_name, sep)

def _add_routes(self, routes: list[Route]):
"""Add routes to the router and index.
Expand Down
11 changes: 7 additions & 4 deletions redisvl/migration/async_planner.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
from redisvl.redis.connection import supports_svs_async
from redisvl.schema.schema import IndexSchema
from redisvl.types import AsyncRedisClient
from redisvl.utils.utils import match_pattern


class AsyncMigrationPlanner:
Expand Down Expand Up @@ -266,17 +267,19 @@ async def _async_sample_keys(
for prefix in prefixes:
if len(key_sample) >= self.key_sample_limit:
break
# NOTE: appending key_separator diverges from the sync planner and
# samples a narrower key set than the index covers. Pre-existing.
if prefix == "":
match_pattern = "*"
scan_match = "*"
elif prefix.endswith(key_separator):
match_pattern = f"{prefix}*"
scan_match = match_pattern(prefix)
else:
match_pattern = f"{prefix}{key_separator}*"
scan_match = match_pattern(prefix, key_separator)
cursor: int = 0
while True:
cursor, keys = await client.scan(
cursor=cursor,
match=match_pattern,
match=scan_match,
count=max(self.key_sample_limit, 10),
)
for key in keys:
Expand Down
7 changes: 4 additions & 3 deletions redisvl/migration/planner.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
)
from redisvl.redis.connection import supports_svs
from redisvl.schema.schema import IndexSchema
from redisvl.utils.utils import match_pattern


class MigrationPlanner:
Expand Down Expand Up @@ -657,18 +658,18 @@ def _sample_keys(
if len(key_sample) >= self.key_sample_limit:
break
if prefix == "":
match_pattern = "*"
scan_match = "*"
else:
# Use literal prefix + glob, matching Redis Search PREFIX
# semantics (pure string-prefix match). Do NOT insert the
# key_separator — a PREFIX of "doc" must match "doc:1",
# "doca:1", etc., exactly like FT.CREATE does.
match_pattern = f"{prefix}*"
scan_match = match_pattern(prefix)
cursor = 0
while True:
cursor, keys = client.scan(
cursor=cursor,
match=match_pattern,
match=scan_match,
count=max(self.key_sample_limit, 1000),
)
for key in keys:
Expand Down
3 changes: 2 additions & 1 deletion redisvl/migration/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from redisvl.redis.connection import RedisConnectionFactory
from redisvl.schema.schema import IndexSchema
from redisvl.utils.log import get_logger
from redisvl.utils.utils import match_pattern

logger = get_logger(__name__)

Expand Down Expand Up @@ -104,7 +105,7 @@ def build_scan_match_patterns(prefixes: List[str], key_separator: str) -> List[s
# (pure string-prefix match). Do NOT insert the key_separator — a
# PREFIX of "doc" must match "doc:1", "doca:1", etc., exactly like
# FT.CREATE does.
patterns.add(f"{prefix}*")
patterns.add(match_pattern(prefix))
return sorted(patterns)


Expand Down
30 changes: 30 additions & 0 deletions redisvl/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,36 @@ def norm_l2_distance(value: float) -> float:
return 1 / (1 + value)


# Redis glob specials (https://redis.io/docs/latest/commands/keys/). ``]``, ``^``
# and ``-`` are absent deliberately: they only bite inside a ``[...]`` class,
# which can never open once ``[`` is escaped.
_GLOB_METACHARACTERS = frozenset("\\*?[")


def match_pattern(*segments: str) -> str:
"""Build a ``SCAN``/``KEYS`` ``MATCH`` pattern from literal key segments.

Every segment -- prefix, separator, cache or route name -- is escaped, so a
name containing glob metacharacters matches its own keys instead of someone
else's. Build patterns here rather than interpolating names into a pattern.

Args:
*segments (str): Literal parts of the key prefix, in order. Passing none
(or only empty ones) yields ``"*"``.

Returns:
str: A pattern matching exactly the keys starting with those segments.
"""
return (
"".join(
"\\" + char if char in _GLOB_METACHARACTERS else char
for segment in segments
for char in segment
)
+ "*"
)


def scan_by_pattern(
redis_client: Redis,
pattern: str,
Expand Down
52 changes: 52 additions & 0 deletions tests/integration/test_embedcache.py
Original file line number Diff line number Diff line change
Expand Up @@ -773,3 +773,55 @@ def test_large_batch_operations(cache):
assert not cache.exists_by_key(key)
else:
assert cache.exists_by_key(key)


@pytest.fixture
def colliding_caches(redis_url, redis_test_name):
"""Three caches whose names collide under an unescaped SCAN glob.

``base[ab]:*`` as a glob matches ``basea:``/``baseb:`` but not
``base[ab]:``, so clearing the bracketed cache used to wipe the other two.
"""
base = redis_test_name("glob_collision")
caches = {
name: EmbeddingsCache(name=name, redis_url=redis_url)
for name in (f"{base}[ab]", f"{base}a", f"{base}b")
}
yield f"{base}[ab]", caches
for cache_instance in caches.values():
cache_instance.clear()


def test_clear_does_not_delete_caches_colliding_under_glob(colliding_caches):
target_name, caches = colliding_caches
for name, cache_instance in caches.items():
cache_instance.set(content=name, model_name="test-model", embedding=[0.1, 0.2])
assert cache_instance.exists(name, "test-model")

caches[target_name].clear()

assert not caches[target_name].exists(target_name, "test-model")
for name, cache_instance in caches.items():
if name != target_name:
assert cache_instance.exists(
name, "test-model"
), f"clear() on {target_name!r} deleted entries belonging to {name!r}"


@pytest.mark.asyncio
async def test_aclear_does_not_delete_caches_colliding_under_glob(colliding_caches):
target_name, caches = colliding_caches
for name, cache_instance in caches.items():
await cache_instance.aset(
content=name, model_name="test-model", embedding=[0.1, 0.2]
)
assert await cache_instance.aexists(name, "test-model")

await caches[target_name].aclear()

assert not await caches[target_name].aexists(target_name, "test-model")
for name, cache_instance in caches.items():
if name != target_name:
assert await cache_instance.aexists(
name, "test-model"
), f"aclear() on {target_name!r} deleted entries belonging to {name!r}"
31 changes: 30 additions & 1 deletion tests/integration/test_migration_v1.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from redisvl.index import SearchIndex
from redisvl.migration import MigrationExecutor, MigrationPlanner, MigrationValidator
from redisvl.migration.utils import load_migration_plan, schemas_equal
from redisvl.redis.utils import array_to_buffer
from redisvl.redis.utils import array_to_buffer, convert_bytes


def test_drop_recreate_plan_apply_validate_flow(redis_url, worker_id, tmp_path):
Expand Down Expand Up @@ -128,3 +128,32 @@ def test_drop_recreate_plan_apply_validate_flow(redis_url, worker_id, tmp_path):
finally:
live_index = SearchIndex.from_existing(index_name, redis_url=redis_url)
live_index.delete(drop=True)


def test_scan_patterns_select_only_the_named_prefix(client, redis_test_name):
"""Redis, not a reimplementation of its glob, decides what a pattern matches.

Exercises the patterns the migration and router builders emit for a prefix
carrying glob metacharacters: unescaped, ``base[ab]*`` selects ``basea``
and ``baseb`` and misses ``base[ab]`` entirely.
"""
from redisvl.migration.utils import build_scan_match_patterns
from redisvl.utils.utils import match_pattern

base = redis_test_name("glob_prefix")
owned = {f"{base}[ab]:1", f"{base}[ab]:2"}
others = {f"{base}a:1", f"{base}b:1"}
for key in owned | others:
client.set(key, "1")

(pattern,) = build_scan_match_patterns([f"{base}[ab]"], ":")
assert set(convert_bytes(list(client.scan_iter(match=pattern)))) == owned

route_pattern = match_pattern(f"{base}[ab]", ":", "route", ":")
client.set(f"{base}[ab]:route:ref", "1")
client.set(f"{base}a:route:ref", "1")
assert convert_bytes(list(client.scan_iter(match=route_pattern))) == [
f"{base}[ab]:route:ref"
]

client.delete(*(owned | others), f"{base}[ab]:route:ref", f"{base}a:route:ref")
Loading
Loading