diff --git a/src/tool_security.py b/src/tool_security.py index 9ebda0a..c4094b9 100644 --- a/src/tool_security.py +++ b/src/tool_security.py @@ -48,9 +48,17 @@ NON_ADMIN_BLOCKED_TOOLS = { def is_public_blocked_tool(tool_name: Optional[str]) -> bool: - """Return True when a non-admin/public user must not execute this tool.""" - if not tool_name: + """Return True when a non-admin/public user must not execute this tool. + + 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 + if not isinstance(tool_name, str): + return True return tool_name in NON_ADMIN_BLOCKED_TOOLS or tool_name.startswith("mcp__") diff --git a/tests/test_public_blocked_tool_nonstring.py b/tests/test_public_blocked_tool_nonstring.py new file mode 100644 index 0000000..64d4114 --- /dev/null +++ b/tests/test_public_blocked_tool_nonstring.py @@ -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