fix: is_public_blocked_tool crashes on a truthy non-string tool name (#1620)
* fix: is_public_blocked_tool crashes on a truthy non-string tool name * fix: is_public_blocked_tool fails closed (blocks) on a malformed non-string tool name
This commit is contained in:
@@ -48,9 +48,17 @@ NON_ADMIN_BLOCKED_TOOLS = {
|
|||||||
|
|
||||||
|
|
||||||
def is_public_blocked_tool(tool_name: Optional[str]) -> bool:
|
def is_public_blocked_tool(tool_name: Optional[str]) -> bool:
|
||||||
"""Return True when a non-admin/public user must not execute this tool."""
|
"""Return True when a non-admin/public user must not execute this tool.
|
||||||
if not tool_name:
|
|
||||||
|
This is a security gate, so it fails CLOSED: a malformed non-string tool
|
||||||
|
name can't be matched against the blocklist or the ``mcp__`` namespace, so
|
||||||
|
it is treated as blocked rather than silently allowed through. ``None`` /
|
||||||
|
empty string means there is no tool to gate.
|
||||||
|
"""
|
||||||
|
if tool_name is None or tool_name == "":
|
||||||
return False
|
return False
|
||||||
|
if not isinstance(tool_name, str):
|
||||||
|
return True
|
||||||
return tool_name in NON_ADMIN_BLOCKED_TOOLS or tool_name.startswith("mcp__")
|
return tool_name in NON_ADMIN_BLOCKED_TOOLS or tool_name.startswith("mcp__")
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
25
tests/test_public_blocked_tool_nonstring.py
Normal file
25
tests/test_public_blocked_tool_nonstring.py
Normal file
@@ -0,0 +1,25 @@
|
|||||||
|
"""Regression: is_public_blocked_tool must fail CLOSED on a non-string tool name.
|
||||||
|
|
||||||
|
The `if not tool_name` guard only handled falsy values; a truthy non-string
|
||||||
|
(e.g. 5 or a list) reached `tool_name.startswith("mcp__")` and raised
|
||||||
|
AttributeError/TypeError. Because this is a public-execution security gate, a
|
||||||
|
malformed (non-string) identifier must be treated as BLOCKED, not silently
|
||||||
|
allowed. None/empty mean there is no tool to gate.
|
||||||
|
"""
|
||||||
|
from src.tool_security import is_public_blocked_tool
|
||||||
|
|
||||||
|
|
||||||
|
def test_malformed_non_string_name_is_blocked():
|
||||||
|
# Fail closed: a non-string identifier cannot be validated, so block it.
|
||||||
|
assert is_public_blocked_tool(5) is True
|
||||||
|
assert is_public_blocked_tool(["bash"]) is True
|
||||||
|
assert is_public_blocked_tool({"x": 1}) is True
|
||||||
|
|
||||||
|
|
||||||
|
def test_none_or_empty_is_not_gated():
|
||||||
|
assert is_public_blocked_tool(None) is False
|
||||||
|
assert is_public_blocked_tool("") is False
|
||||||
|
|
||||||
|
|
||||||
|
def test_real_tool_names_still_classified():
|
||||||
|
assert is_public_blocked_tool("mcp__whatever") is True
|
||||||
Reference in New Issue
Block a user