Skip to content

feat: add MCP_REDIS_ALLOWED_TOOLS to expose only the named tools - #161

Open
cyb3ralbert wants to merge 1 commit into
redis:mainfrom
cyb3ralbert:allowed-tools
Open

feat: add MCP_REDIS_ALLOWED_TOOLS to expose only the named tools#161
cyb3ralbert wants to merge 1 commit into
redis:mainfrom
cyb3ralbert:allowed-tools

Conversation

@cyb3ralbert

@cyb3ralbert cyb3ralbert commented Jul 28, 2026

Copy link
Copy Markdown

Problem

Every tool is registered unconditionally. load_tools() imports each module under src/tools/, the @mcp.tool() decorators run, and all 53 tools land in tools/list — including delete, expire, rename, json_del, hdel, xdel, srem, zrem, lrem, xgroup_destroy and the rest of the write surface. There is no way to hand an agent a subset.

The control the README currently points at is Redis ACL:

ACL SETUSER readonlyuser on >mypassword ~* +@read -@write

That restricts what may be executed, not what is offered. Two consequences:

  1. The tool stays in the inventory. The agent sees delete, plans a step around it, calls it, and gets Error deleting key foo: NOPERM ... back as a tool result. The refusal arrives after the plan was made, not before.
  2. ACL granularity is over Redis commands, not MCP tools, and it is unavailable to anyone who does not administer the Redis server — managed and shared instances included.

Change

MCP_REDIS_ALLOWED_TOOLS takes a comma-separated list of tool names and exposes only those:

MCP_REDIS_ALLOWED_TOOLS="get,set,hget,hgetall,dbsize" redis-mcp-server --url redis://localhost:6379/0
tools/list = 53  →  tools/list = 5

Three properties, in the order they matter:

Unregistered, not refused. Tools outside the list are removed via the public FastMCP.remove_tool, so they are absent from tools/list rather than failing on call. A tool an agent cannot see is a tool it cannot plan around.

Allowlist, not denylist. A tool added in a future release stays unavailable until it is named. A denylist would ship new write tools to existing users on upgrade.

Unknown names abort startup. A misspelled name would otherwise be dropped silently, leaving fewer tools than configured — indistinguishable, from the outside, from the agent simply not using the tool. The error names the offending entries and lists what is available:

ValueError: MCP_REDIS_ALLOWED_TOOLS names unknown tool: gett.
Available tools: client_list, create_vector_index_hash, dbsize, delete, ...

Leaving the variable unset changes nothing — all 53 tools stay available, so this is inert for existing users.

Tests

tests/test_tool_allowlist.py, 16 cases across three groups: parsing (unset vs. empty vs. whitespace vs. stray commas), resolution against the registered set (subset, full set, empty, one typo, several typos sorted, error contents), and application to a real FastMCP instance asserted through list_tools() — including that an unknown name raises and leaves the server untouched.

Verified locally against the checks in .github/workflows/ci.yml: full suite 361 passed, coverage 88.46% (gate 80%), ruff format --check clean, bandit -r src/ clean, and the startup smoke test passes. End-to-end runs confirm 53 → 3 tool inventories and the abort-on-typo path through src/main.py.

Notes

Two things I would rather you decide than assume:

apply_tool_allowlist reads server._tool_manager.list_tools() to enumerate what is registered. FastMCP exposes no synchronous accessor — list_tools() is a coroutine and this runs at import time. Happy to switch to asyncio.run or to track names at registration if you prefer to avoid the private attribute.

The failure on an unknown name is a ValueError raised at import, so it surfaces as a traceback rather than a click error, because src/common/server.py is imported before cli() runs. If you would rather it print through click.echo(..., err=True) and sys.exit(1) like the URI parsing error in main.py, that needs the import restructured and I did not want to do that unasked.

A read-only baseline keyword (read-only plus named write tools, rather than naming every read tool) is the natural follow-up and is how I implemented the equivalent in chigwell/telegram-mcp#168. It needs each of the 53 tools classified, which is a judgement call per tool, so I left it out of this PR rather than bundle it. Glad to add it if you want it.


Note

Medium Risk
Opt-in env var with safe default, but misconfiguration can strip the tool surface or block startup; allowlist logic runs at import time using FastMCP internals.

Overview
Adds MCP_REDIS_ALLOWED_TOOLS, a comma-separated allowlist of MCP tool names. After all tools load, the server unregisters anything not on the list so omitted tools never show up in tools/list (unlike Redis ACL, which only blocks execution).

Unset env leaves behavior unchanged (all tools). An empty string exposes no tools. Misspelled or unknown names raise ValueError at import/startup and list available tools instead of being ignored.

README gains a Restricting the exposed tools section (vs ACL), env table entry, and client config example. tests/test_tool_allowlist.py covers parsing, resolution, and FastMCP application via list_tools().

Reviewed by Cursor Bugbot for commit 02b03b8. Bugbot is set up for automated code reviews on this repo. Configure here.

The server registers all 53 tools unconditionally, so an agent is always
offered `delete`, `expire`, `rename` and the rest of the write surface.
The only control the README can point at is Redis ACL, which restricts
what may be executed rather than what is offered: a denied command still
comes back as a tool error once the agent has decided to call it, and
ACL granularity is over Redis commands, not MCP tools.

Add an allowlist of tool names. Tools outside it are unregistered, so
they are absent from tools/list rather than refused on call — a visible
tool can be planned around even when every call to it would fail.

Being an allowlist rather than a denylist, a tool added in a future
release stays unavailable until it is named. An unknown name aborts
startup instead of being dropped, which would otherwise expose fewer
tools than configured while looking like a working configuration.

Leaving the variable unset keeps every tool available.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant