Repository navigation
feat(mcp): add an allow-listed index handle and ToolContext for code tools - #758
vishal-bala wants to merge 4 commits into
Conversation
…tools A code tool reaches Redis only through a ToolContext. Its index is a handle that spells out every method it offers and routes each one through run_guarded, so the concurrency cap and the binding's request timeout apply to every call. Write methods exist only on the handle given to a tool that declares it writes, and exclude the filter-wide bulk operations a timeout could cancel partway. Claims are a deep read-only copy of the verified token.
The write methods took full Redis keys, so a tool bound to one index could write, delete or expire keys belonging to any other, including a tenant-scoped one. Records are now addressed only by document id, which the index turns into a key under its own prefix: load no longer takes keys, and drop_keys and expire_keys give way to drop_documents and expire_documents. query and batch_query refuse a SQLQuery, whose FROM clause rather than the bound index decides what it reads. Each request also gets its own copy of the schema, which the index shares, so a tool that edits it cannot retarget later calls.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0ff72fe. Configure here.
| preprocess=preprocess, | ||
| batch_size=batch_size, | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Load keys cannot address documents
Medium Severity
load still returns full Redis keys from AsyncSearchIndex.load, while fetch, drop_documents, and expire_documents only accept document ids and re-apply the index prefix. Passing those returned keys back in double-prefixes, so expire and drop silently miss the records just written. That is especially broken when id_field is omitted, because the generated ULID keys are the only identifiers the tool receives. Query and search results have the same full-key id field, so the natural query-then-drop path fails the same way. The real index offers drop_keys for this; the handle removed that on purpose and left no id-based replacement for load's return value.
Additional Locations (2)
Triggered by project rule: Bugbot review guide for RedisVL
Reviewed by Cursor Bugbot for commit 0ff72fe. Configure here.
Motivation
Custom tool profiles cover what YAML can express: a built-in search with some arguments frozen. Logic beyond that, such as re-ranking results, counting matches or combining several queries, needs Python. This stack adds code tools: async functions registered as MCP tools. This first PR adds the only thing such a function will receive to reach Redis, a
ToolContext. Nothing in it is reachable by users yet.The design point is that a code tool never holds the real
AsyncSearchIndex, the server or a Redis client. The real index has write methods,create/delete/clearand a.clientproperty, so handing it over would make every promise about read-only policy, resource limits and index scoping depend on the tool author's discipline.Changes
redisvl/mcp/context.pyadds three classes:IndexHandleoffersquery,batch_query,search,batch_search,aggregate,fetch,existsandinfo. Every method is spelled out rather than forwarded through__getattr__, so a method added toAsyncSearchIndexlater stays unreachable until someone adds it here on purpose.WritableIndexHandleaddsload,drop_documentsandexpire_documents, for tools that declare they write.ToolContextbundles the handle with a copy of the binding's schema, its vectorizer, anembed(text)helper andclaims, a deep read-only copy of the current request's verified token claims, orNonewithout a token.Every handle method routes through
server.run_guarded, so each Redis call takes a slot under the process-wide concurrency cap and is bounded by the binding's request timeout. The cap is taken per call, not per tool, so a tool that queries several times never blocks itself.Every call stays inside the bound index:
infotakes no index name.loadhas nokeysargument, and the handle offersdrop_documentsandexpire_documentsrather thandrop_keysandexpire_keys.queryandbatch_queryrefuse aSQLQuery, because itsFROMclause, not the bound index, decides what it reads.Without these, a tool bound to one index could read or rewrite another's documents, including a tenant-scoped one.
The writable handle leaves out
update_by_filteranddrop_by_filter. Both work through their matches in batches, so a request timeout could cancel either partway and leave a partial write.Notes
The handle is a guardrail, not a sandbox. A code tool is the operator's own code, and Python can still reach private attributes. The aim is to make the bounded, policy-checked path the only obvious one.
An index whose key prefix overlaps another's shares keys with it. The handle addresses keys through the index's prefix, so it cannot tell such indexes apart. An index with an empty prefix covers every key, so its document ids are full Redis keys.
ToolContext.schemais a copy because the index holds the same object. A tool that edited a shared schema would retarget every later call on the binding, built-in tools included.The tests pin the exact public method sets of both handles. They also check every forwarded call against the real
AsyncSearchIndexsignature, so a renamed upstream parameter fails here rather than at a tool's first call.Release Notes
Code tools reach Redis only through a
ToolContext, whose index handle stays inside the bound index, applies the concurrency cap and request timeout to every call, and offers writes, addressed by document id, only to tools that declare them.Next Steps
Stacked PRs build on this one: the tool wrapper that constructs a
ToolContextper call, thenserver.code_tool(...), then the module loader.Review record
Reviewed at
bd7364cby Claude Code (claude-opus-5-5) withpre-pr-review; fixes in0ff72fe.Lenses: correctness (always); security (new
redisvl/mcp/surface that must stay inside one index); design (new publicToolContextAPI); tests (adds unit tests); docs (module and method docstrings); Redis behaviour (key prefixes andEXPIREsemantics). The lenses ran over the whole stack, and findings were attributed to the PR that introduced the line.loadtakes nokeys; a conditionalkeys=passthrough passed every test (test_tool_context.py:217, now a parametrised "never takes another target" test).context.py:17) and, in feat(mcp): register code tools from Python with server.code_tool #760, the guides.expire_documentswithttl <= 0deletes the records (this is howEXPIREbehaves, inherited unchanged fromexpire_keys, and the value only comes from the model if the tool author passes it through);context.pyimporting_embed_queryfromtools/search.py(consistent with the built-ins, with one caller).Verified:
pytest tests/unit/test_mcp(512 passed);pytest -n 1 tests/integration/test_mcp(80 passed, 2 skipped, Redis 8).Docs consulted: https://redis.io/docs/latest/commands/ft.create/; https://redis.io/docs/latest/commands/expire/; https://redis.io/docs/latest/commands/ft.info/.
Not checked: cluster mode, where
drop_documentsandexpire_documentsacross hash slots behave as the existingAsyncSearchIndexmethods do; FastMCP 2.x, which the>=2.0.0floor allows (tested on 3.1.0 and 3.3.1).Note
Medium Risk
New security-sensitive MCP API that defines index scoping and write policy for arbitrary Python tools; not wired to user-facing registration yet, but mistakes here would affect all code tools.
Overview
Introduces the only Redis entry point future MCP code tools will get: a
ToolContextbuilt per request viaToolContext.for_request, instead of handing tools the realAsyncSearchIndex, server, or Redis client.IndexHandleexposes an explicit allow-list of read operations (query,search,fetch, etc.); each call goes throughserver.run_guardedwith the binding’s request timeout.WritableIndexHandleadds bounded writes (load,drop_documents,expire_documents) only whenwrites=True. Cross-index escape hatches are blocked:SQLQueryis rejected, APIs use document ids (not raw keys or alternate index names), and filter-wide bulk writes are omitted.The context also supplies a deep-copied schema, the binding vectorizer with
embed(), and deep-frozen JWT claims (orNone). Unit tests pin the public handle surface, verify forwarding through the guard, and assert scoping and immutability behavior.Reviewed by Cursor Bugbot for commit 0ff72fe. Bugbot is set up for automated code reviews on this repo. Configure here.