From 0e1c1603d17d76ccd532adca4d4fac9f36c8c987 Mon Sep 17 00:00:00 2001 From: Vita Stejskal Date: Wed, 18 Feb 2026 18:18:42 +0100 Subject: [PATCH 1/3] AI-2442: add read-only access control for guest and read-only roles - Filter tools list to read-only tools only for guest/readonly roles - Block write tool calls for guest/readonly roles with a clear error message - Swallow 403 errors in tool event trigger (expected for restricted roles) - Extract is_read_only_tool() to utils.py, removing duplicate from authorization.py - Bump version to 1.44.8 Co-Authored-By: Claude Sonnet 4.6 --- pyproject.toml | 2 +- src/keboola_mcp_server/authorization.py | 11 +---- src/keboola_mcp_server/errors.py | 10 ++++- src/keboola_mcp_server/mcp.py | 21 ++++++++++ tests/test_mcp.py | 55 ++++++++++++++++--------- uv.lock | 2 +- 6 files changed, 70 insertions(+), 31 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index e3c7f14b..aaca9978 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "keboola-mcp-server" -version = "1.44.7" +version = "1.44.8" description = "MCP server for interacting with Keboola Connection" readme = "README.md" requires-python = ">=3.10" diff --git a/src/keboola_mcp_server/authorization.py b/src/keboola_mcp_server/authorization.py index 7da923a4..15857edd 100644 --- a/src/keboola_mcp_server/authorization.py +++ b/src/keboola_mcp_server/authorization.py @@ -22,7 +22,7 @@ from fastmcp.tools import Tool from mcp import types as mt -from keboola_mcp_server.mcp import get_http_request_or_none +from keboola_mcp_server.mcp import get_http_request_or_none, is_read_only_tool LOG = logging.getLogger(__name__) @@ -81,13 +81,6 @@ def _get_authorization_config() -> tuple[set[str] | None, set[str] | None, bool] return allowed_tools, disallowed_tools, read_only_mode - @staticmethod - def _is_read_only_tool(tool: Tool) -> bool: - """Check if a tool has readOnlyHint=True annotation.""" - if tool.annotations is None: - return False - return tool.annotations.readOnlyHint is True - @staticmethod def _is_tool_authorized( tool: Tool, allowed_tools: set[str] | None, disallowed_tools: set[str] | None, read_only_mode: bool @@ -97,7 +90,7 @@ def _is_tool_authorized( if disallowed_tools and tool.name in disallowed_tools: return False # Check read-only mode - only allow tools with readOnlyHint=True - if read_only_mode and not ToolAuthorizationMiddleware._is_read_only_tool(tool): + if read_only_mode and not is_read_only_tool(tool): return False # Then check if tool is in allowed list (if specified) if allowed_tools is not None and tool.name not in allowed_tools: diff --git a/src/keboola_mcp_server/errors.py b/src/keboola_mcp_server/errors.py index ca9143d2..270b66b2 100644 --- a/src/keboola_mcp_server/errors.py +++ b/src/keboola_mcp_server/errors.py @@ -5,6 +5,7 @@ from functools import wraps from typing import Any, Callable, Mapping, Optional, Type, TypeVar, cast +import httpx import yaml from fastmcp import Context from fastmcp.exceptions import ToolError @@ -170,7 +171,14 @@ async def wrapped(*args, **kwargs): await _trigger_event(func, args, kwargs, exception, time.perf_counter() - start) except Exception as e: LOG.exception(f'Failed to trigger tool event for "{func.__name__}" tool: {e}') - raise + # Only swallow 403 Forbidden errors (expected for guest/read-only roles) + # Re-raise other errors as they indicate genuine problems + if isinstance(e, httpx.HTTPStatusError) and e.response.status_code == 403: + # Expected for restricted roles (guest, read-only) - don't fail the tool call + pass + else: + # Unexpected error - re-raise to alert about the problem + raise return cast(F, wrapped) diff --git a/src/keboola_mcp_server/mcp.py b/src/keboola_mcp_server/mcp.py index e4d35deb..7d35b7ab 100644 --- a/src/keboola_mcp_server/mcp.py +++ b/src/keboola_mcp_server/mcp.py @@ -44,6 +44,13 @@ DEFAULT_CONCURRENCY = 10 +def is_read_only_tool(tool: Tool) -> bool: + """Check if a tool has readOnlyHint=True annotation.""" + if tool.annotations is None: + return False + return tool.annotations.readOnlyHint is True + + @dataclasses.dataclass(frozen=True) class ServerState: config: Config @@ -330,6 +337,11 @@ async def on_list_tools( # Filter out data app tools when the client is not using the main/production branch tools = [t for t in tools if t.name not in {'modify_data_app', 'get_data_apps', 'deploy_data_app'}] + # Role-based filtering: read-only access for guest and read roles + if token_role in ['guest', 'readonly']: + tools = [t for t in tools if is_read_only_tool(t)] + LOG.debug(f'Read-only access: filtered to {len(tools)} read-only tools for role={token_role}') + return tools async def on_call_tool( @@ -342,6 +354,15 @@ async def on_call_tool( features = self.get_project_features(token_info) token_role = self.get_token_role(token_info).lower() + # Block non-read-only tools for guest and read-only roles + if token_role in ['guest', 'readonly']: + if not is_read_only_tool(tool): + raise ToolError( + f'Access denied: The tool "{tool.name}" requires write permissions. ' + f'Your current role ({token_role}) only allows read-only operations. ' + f'Contact your administrator to request write access.' + ) + if 'hide-conditional-flows' in features: if tool.name == 'create_conditional_flow': raise ToolError( diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 8a8f0b58..19833cbb 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -34,9 +34,13 @@ class NestedModel(BaseModel): field2: list[str] | None = None -def _tool(name: str) -> MagicMock: +def _tool(name: str, read_only: bool = False) -> MagicMock: tool = MagicMock() tool.name = name + if read_only: + tool.annotations.readOnlyHint = True + else: + tool.annotations = None return tool @@ -410,27 +414,33 @@ async def call_next(_): @pytest.mark.asyncio @pytest.mark.parametrize( - ('token_role', 'hidden_tool', 'visible_tool'), + ('token_role', 'hidden_tools', 'visible_tools'), [ - ('admin', 'update_flow', 'modify_flow'), - ('share', 'update_flow', 'modify_flow'), - ('', 'modify_flow', 'update_flow'), - ('readOnly', 'modify_flow', 'update_flow'), + ('admin', {'update_flow'}, {'modify_flow'}), + ('share', {'update_flow'}, {'modify_flow'}), + ('', {'modify_flow'}, {'update_flow'}), + ('readOnly', {'modify_flow', 'update_flow'}, {'read_only_tool'}), + ('guest', {'modify_flow', 'update_flow'}, {'read_only_tool'}), ], ) async def test_list_tools_filters_flow_tools_by_role( self, mcp_context_client, token_role: str, - hidden_tool: str, - visible_tool: str, + hidden_tools: set[str], + visible_tools: set[str], ) -> None: keboola_client = KeboolaClient.from_state(mcp_context_client.session.state) keboola_client.storage_client.verify_token = AsyncMock( return_value={'owner': {'features': []}, 'admin': {'role': token_role}} ) - tools = [_tool(hidden_tool), _tool(visible_tool), _tool('other_tool')] + tools = [ + _tool('modify_flow'), + _tool('update_flow'), + _tool('other_tool'), + _tool('read_only_tool', read_only=True), + ] async def call_next(_): return tools @@ -440,19 +450,25 @@ async def call_next(_): result = await middleware.on_list_tools(context, call_next) result_names = {t.name for t in result} - assert hidden_tool not in result_names - assert visible_tool in result_names + for tool_name in hidden_tools: + assert tool_name not in result_names + for tool_name in visible_tools: + assert tool_name in result_names @pytest.mark.asyncio @pytest.mark.parametrize( - ('token_role', 'called_tool', 'expect_error'), + ('token_role', 'called_tool', 'tool_read_only', 'expect_error'), [ - ('admin', 'modify_flow', False), - ('admin', 'update_flow', True), - ('share', 'modify_flow', False), - ('share', 'update_flow', True), - ('', 'modify_flow', True), - ('', 'update_flow', False), + ('admin', 'modify_flow', False, False), + ('admin', 'update_flow', False, True), + ('share', 'modify_flow', False, False), + ('share', 'update_flow', False, True), + ('', 'modify_flow', False, True), + ('', 'update_flow', False, False), + ('guest', 'write_tool', False, True), + ('guest', 'read_only_tool', True, False), + ('readOnly', 'write_tool', False, True), + ('readOnly', 'read_only_tool', True, False), ], ) async def test_call_tool_blocks_flow_tools_by_role( @@ -460,6 +476,7 @@ async def test_call_tool_blocks_flow_tools_by_role( mcp_context_client, token_role: str, called_tool: str, + tool_read_only: bool, expect_error: bool, ) -> None: keboola_client = KeboolaClient.from_state(mcp_context_client.session.state) @@ -467,7 +484,7 @@ async def test_call_tool_blocks_flow_tools_by_role( return_value={'owner': {'features': []}, 'admin': {'role': token_role}} ) - tool = _tool(called_tool) + tool = _tool(called_tool, read_only=tool_read_only) mcp_context_client.fastmcp = SimpleNamespace(get_tool=AsyncMock(return_value=tool)) context = SimpleNamespace(fastmcp_context=mcp_context_client, message=SimpleNamespace(name=called_tool)) diff --git a/uv.lock b/uv.lock index 240c20f4..962d3cb7 100644 --- a/uv.lock +++ b/uv.lock @@ -1223,7 +1223,7 @@ wheels = [ [[package]] name = "keboola-mcp-server" -version = "1.44.7" +version = "1.44.8" source = { editable = "." } dependencies = [ { name = "cryptography" }, From 109fbce7c8f4175a73a4f5b170819da968d93296 Mon Sep 17 00:00:00 2001 From: Vita Stejskal Date: Fri, 20 Feb 2026 08:17:23 +0100 Subject: [PATCH 2/3] AI-2442: let 'guest' role access all tools except of those for admins only --- src/keboola_mcp_server/mcp.py | 8 +++----- tests/test_mcp.py | 10 +++++----- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/src/keboola_mcp_server/mcp.py b/src/keboola_mcp_server/mcp.py index 7d35b7ab..c1d0ddff 100644 --- a/src/keboola_mcp_server/mcp.py +++ b/src/keboola_mcp_server/mcp.py @@ -337,8 +337,7 @@ async def on_list_tools( # Filter out data app tools when the client is not using the main/production branch tools = [t for t in tools if t.name not in {'modify_data_app', 'get_data_apps', 'deploy_data_app'}] - # Role-based filtering: read-only access for guest and read roles - if token_role in ['guest', 'readonly']: + if token_role == 'readonly': tools = [t for t in tools if is_read_only_tool(t)] LOG.debug(f'Read-only access: filtered to {len(tools)} read-only tools for role={token_role}') @@ -354,8 +353,7 @@ async def on_call_tool( features = self.get_project_features(token_info) token_role = self.get_token_role(token_info).lower() - # Block non-read-only tools for guest and read-only roles - if token_role in ['guest', 'readonly']: + if token_role == 'readonly': if not is_read_only_tool(tool): raise ToolError( f'Access denied: The tool "{tool.name}" requires write permissions. ' @@ -391,7 +389,7 @@ async def on_call_tool( f'Use "{UPDATE_FLOW_TOOL_NAME}" to update flow configuration instead.' ) - if tool.name in {'modify_data_app', 'get_data_apps', 'deploy_data_app'}: + if tool.name in ('modify_data_app', 'get_data_apps', 'deploy_data_app'): if not self.is_client_using_main_branch(context.fastmcp_context): raise ToolError('Data apps are supported only in the main production branch.') diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 19833cbb..dd1798af 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -416,11 +416,11 @@ async def call_next(_): @pytest.mark.parametrize( ('token_role', 'hidden_tools', 'visible_tools'), [ - ('admin', {'update_flow'}, {'modify_flow'}), - ('share', {'update_flow'}, {'modify_flow'}), - ('', {'modify_flow'}, {'update_flow'}), + ('admin', {'update_flow'}, {'modify_flow', 'read_only_tool'}), + ('share', {'update_flow'}, {'modify_flow', 'read_only_tool'}), + ('', {'modify_flow'}, {'update_flow', 'read_only_tool'}), ('readOnly', {'modify_flow', 'update_flow'}, {'read_only_tool'}), - ('guest', {'modify_flow', 'update_flow'}, {'read_only_tool'}), + ('guest', {'modify_flow'}, {'update_flow', 'read_only_tool'}), ], ) async def test_list_tools_filters_flow_tools_by_role( @@ -465,7 +465,7 @@ async def call_next(_): ('share', 'update_flow', False, True), ('', 'modify_flow', False, True), ('', 'update_flow', False, False), - ('guest', 'write_tool', False, True), + ('guest', 'write_tool', False, False), ('guest', 'read_only_tool', True, False), ('readOnly', 'write_tool', False, True), ('readOnly', 'read_only_tool', True, False), From 6f8c9d304c290b99ceec7bcf08a6a62dce23a405 Mon Sep 17 00:00:00 2001 From: Vita Stejskal Date: Sat, 21 Feb 2026 21:22:09 +0100 Subject: [PATCH 3/3] AI-2442: remove implicit requests dependency from cli.py requests.JSONDecodeError is identical to json.JSONDecodeError (requests re-exports it), making the handler redundant. Removing it eliminates the undeclared dependency on the requests library, which is no longer reliably available as a transitive dep (jsonschema-path 0.4.x made it optional). Co-Authored-By: Claude Sonnet 4.6 --- src/keboola_mcp_server/cli.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/keboola_mcp_server/cli.py b/src/keboola_mcp_server/cli.py index b64d3ac1..f57a6c16 100644 --- a/src/keboola_mcp_server/cli.py +++ b/src/keboola_mcp_server/cli.py @@ -12,7 +12,6 @@ from typing import Optional import pydantic -import requests from fastmcp import FastMCP from starlette.exceptions import HTTPException from starlette.middleware import Middleware @@ -95,7 +94,6 @@ async def _http_exception_handler(request: Request, exc: HTTPException): _exception_handlers = { HTTPException: _http_exception_handler, json.JSONDecodeError: _bad_request_handler, - requests.JSONDecodeError: _bad_request_handler, pydantic.ValidationError: _bad_request_handler, ValueError: _bad_request_handler, Exception: _create_exception_handler(status_code=500, log_exception=True),