From 96fbe78e28790d89691b9f8871fd61a5913d8a22 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 31 Aug 2026 14:26:05 +0100 Subject: [PATCH 1/2] fix: bound hosted binary tool responses --- src/mcp_server_appwrite/constants.py | 3 + .../error_classification.py | 35 +++- src/mcp_server_appwrite/server.py | 153 +++++++++++++- src/mcp_server_appwrite/service.py | 15 +- tests/unit/test_error_classification.py | 5 + tests/unit/test_server.py | 198 +++++++++++++++++- tests/unit/test_service.py | 15 ++ 7 files changed, 418 insertions(+), 6 deletions(-) diff --git a/src/mcp_server_appwrite/constants.py b/src/mcp_server_appwrite/constants.py index e4aee10..09ddbc8 100644 --- a/src/mcp_server_appwrite/constants.py +++ b/src/mcp_server_appwrite/constants.py @@ -49,6 +49,9 @@ def _resolve_server_version() -> str: EXCLUDED_SERVICES: frozenset[str] = frozenset() MAX_FETCH_BYTES = 25 * 1024 * 1024 # 25 MB cap on server-fetched files +# MCP embeds binary tool results as base64 in one JSON-RPC response. Bound the +# source bytes before base64 and JSON encoding create additional in-memory copies. +MAX_HOSTED_BINARY_RESPONSE_BYTES = 25 * 1024 * 1024 # Match Cloud/agent chat attachment max (10 MB). Hosted uploads resolve # turn attachments to inline base64; keep this at least that large. MAX_INLINE_BYTES = 10 * 1024 * 1024 # 10 MB cap on decoded inline content diff --git a/src/mcp_server_appwrite/error_classification.py b/src/mcp_server_appwrite/error_classification.py index c4c5b57..66d1141 100644 --- a/src/mcp_server_appwrite/error_classification.py +++ b/src/mcp_server_appwrite/error_classification.py @@ -7,8 +7,9 @@ from __future__ import annotations +import json from collections.abc import Iterator -from typing import Literal +from typing import Any, Literal from appwrite_console.exception import AppwriteException @@ -17,6 +18,7 @@ "appwrite_4xx", "appwrite_5xx", "sdk_validation", + "response_too_large", "internal", ] @@ -26,6 +28,7 @@ "appwrite_4xx", "appwrite_5xx", "sdk_validation", + "response_too_large", "internal", } ) @@ -35,6 +38,33 @@ class WriteConfirmationRequired(RuntimeError): """A mutating hidden tool was called without explicit confirmation.""" +class HostedBinaryResponseTooLarge(ValueError): + """A binary Appwrite response exceeded the hosted MCP memory-safe limit.""" + + def __init__( + self, + tool_name: str, + limit_bytes: int, + *, + content_length: int | None = None, + observed_bytes: int | None = None, + ) -> None: + error: dict[str, Any] = { + "code": "hosted_response_too_large", + "tool": tool_name, + "limitBytes": limit_bytes, + "message": ( + "The binary response is too large to return through hosted MCP. " + "Use an Appwrite SDK or REST API for larger content." + ), + } + if content_length is not None: + error["contentLength"] = content_length + if observed_bytes is not None: + error["observedBytes"] = observed_bytes + super().__init__(json.dumps({"error": error}, separators=(",", ":"))) + + def classify_tool_error(exc: BaseException) -> ErrorCategory: """Return the bounded operational category for an exception chain.""" chain = tuple(_exception_chain(exc)) @@ -42,6 +72,9 @@ def classify_tool_error(exc: BaseException) -> ErrorCategory: if any(isinstance(item, WriteConfirmationRequired) for item in chain): return "write_confirmation" + if any(isinstance(item, HostedBinaryResponseTooLarge) for item in chain): + return "response_too_large" + if any(_is_sdk_validation_error(item) for item in chain): return "sdk_validation" diff --git a/src/mcp_server_appwrite/server.py b/src/mcp_server_appwrite/server.py index 80a4176..fd0693a 100644 --- a/src/mcp_server_appwrite/server.py +++ b/src/mcp_server_appwrite/server.py @@ -60,6 +60,7 @@ FETCH_TIMEOUT_SECONDS, HOSTED_PATH_GUIDANCE, MAX_FETCH_BYTES, + MAX_HOSTED_BINARY_RESPONSE_BYTES, MAX_INLINE_BYTES, SERVER_ICON_URL, SERVER_VERSION, @@ -73,7 +74,7 @@ get_appwrite_context, ) from .docs_search import DocsSearch -from .error_classification import is_response_parse_error +from .error_classification import HostedBinaryResponseTooLarge, is_response_parse_error from .operator import Operator, _parse_tool_name from .service import Service from .tool_manager import ToolManager @@ -381,6 +382,11 @@ def register_services( name, allowed_methods=allowed_methods, context_scope=context_scope(name), + binary_response_limit=( + MAX_HOSTED_BINARY_RESPONSE_BYTES + if profile == OAUTH_PROFILE + else None + ), ) ) return tools_manager @@ -820,6 +826,123 @@ def _prepare_arguments(tool_info: dict, arguments: dict[str, Any]) -> dict[str, return prepared_arguments +def _raise_bounded_response_error(response: httpx.Response) -> None: + """Translate an upstream streaming error into the SDK's public exception.""" + body = bytearray() + for chunk in response.iter_bytes(): + remaining = MAX_INLINE_BYTES - len(body) + if remaining <= 0: + break + body.extend(chunk[:remaining]) + text = bytes(body).decode("utf-8", errors="replace") + message = text or response.reason_phrase + error_type = None + try: + payload = json.loads(text) + if isinstance(payload, dict): + message = str(payload.get("message") or message) + raw_type = payload.get("type") + error_type = str(raw_type) if raw_type is not None else None + except (TypeError, ValueError): + pass + raise AppwriteException(message, response.status_code, error_type, text) + + +def _perform_bounded_binary_client_call( + client: Client, + tool_name: str, + method: str, + path: str = "", + headers: dict[str, Any] | None = None, + params: dict[str, Any] | None = None, + response_type: str = "json", +) -> bytes: + """Stream one SDK binary call into a bounded buffer for hosted HTTP.""" + if method.lower() != "get" or response_type != "json": + raise RuntimeError(f"Unsupported bounded binary request for {tool_name}.") + + request_headers = { + key: value + for key, value in {**client._global_headers, **(headers or {})}.items() + if value + } + # Prevent HTTPX from transparently inflating a compressed response into one + # oversized chunk before the decoded-byte limit can run. + request_headers["accept-encoding"] = "identity" + request_params = client.flatten(params or {}) + endpoint = client._endpoint.rstrip("/") + + with httpx.Client( + verify=not client._self_signed, + timeout=FETCH_TIMEOUT_SECONDS, + follow_redirects=True, + ) as http_client: + with http_client.stream( + method, endpoint + path, headers=request_headers, params=request_params + ) as response: + if response.status_code >= 400: + _raise_bounded_response_error(response) + + content_encoding = response.headers.get("content-encoding", "identity") + if content_encoding.lower().strip() not in {"", "identity"}: + raise ValueError( + "Hosted MCP cannot safely return a compressed binary response. " + "Use an Appwrite SDK or REST API for this content." + ) + + warning = response.headers.get("x-appwrite-warning") + if warning: + for item in warning.split(";"): + print(f"Warning: {item}", file=sys.stderr) + + declared = response.headers.get("content-length") + if declared: + try: + content_length = int(declared) + except ValueError: + content_length = None + if ( + content_length is not None + and content_length > MAX_HOSTED_BINARY_RESPONSE_BYTES + ): + raise HostedBinaryResponseTooLarge( + tool_name, + MAX_HOSTED_BINARY_RESPONSE_BYTES, + content_length=content_length, + ) + + body = bytearray() + for chunk in response.iter_bytes(): + observed_bytes = len(body) + len(chunk) + if observed_bytes > MAX_HOSTED_BINARY_RESPONSE_BYTES: + raise HostedBinaryResponseTooLarge( + tool_name, + MAX_HOSTED_BINARY_RESPONSE_BYTES, + observed_bytes=observed_bytes, + ) + body.extend(chunk) + return bytes(body) + + +def _bounded_binary_client_call( + client: Client, + tool_name: str, + method: str, + path: str = "", + headers: dict[str, Any] | None = None, + params: dict[str, Any] | None = None, + response_type: str = "json", +) -> bytes: + try: + return _perform_bounded_binary_client_call( + client, tool_name, method, path, headers, params, response_type + ) + except httpx.HTTPError as exc: + # Match the generated SDK contract so callers receive the existing + # Appwrite-formatted tool error instead of an internal HTTPX exception. + raise AppwriteException(str(exc)) from exc + + def execute_registered_tool( tools_manager: ToolManager, name: str, @@ -843,13 +966,39 @@ def execute_registered_tool( # Re-bind the SDK method to a client authenticated for the current request. # An explicit client takes precedence (used by tests); otherwise it is resolved # from the request's OAuth access token. + hosted = client is None if client is None: client = resolve_client(target_project, organization_id) bound_method = getattr(service_cls(client), method_name) + bounded_binary = ( + hosted and inspect.signature(bound_method).return_annotation is bytes + ) parsed = _parse_tool_name(name) try: - result = bound_method(**prepared_arguments) + if bounded_binary: + original_call = client.call + setattr( + client, + "call", + lambda method, path="", headers=None, params=None, response_type="json": _bounded_binary_client_call( + client, + name, + method, + path, + headers, + params, + response_type, + ), + ) + try: + result = bound_method(**prepared_arguments) + finally: + setattr(client, "call", original_call) + else: + result = bound_method(**prepared_arguments) + except HostedBinaryResponseTooLarge: + raise except AppwriteException as exc: error_monitoring.capture_appwrite_exception( exc, diff --git a/src/mcp_server_appwrite/service.py b/src/mcp_server_appwrite/service.py index 6965e06..03df2a6 100644 --- a/src/mcp_server_appwrite/service.py +++ b/src/mcp_server_appwrite/service.py @@ -23,11 +23,13 @@ def __init__( *, allowed_methods: frozenset[str] | None = None, context_scope: str = "console", + binary_response_limit: int | None = None, ): self.service = service_instance self.service_name = service_name self.allowed_methods = allowed_methods self.context_scope = context_scope + self.binary_response_limit = binary_response_limit self._method_name_overrides = self.get_method_name_overrides() def get_method_name_overrides(self) -> Dict[str, str]: @@ -204,9 +206,20 @@ def list_tools(self) -> Dict[str, Dict]: if param.default is param.empty: required.append(param_name) + description = docstring.short_description or "No description available" + if ( + self.binary_response_limit is not None + and type_hints.get("return") is bytes + ): + limit_mib = self.binary_response_limit // (1024 * 1024) + description = ( + f"{description} Hosted MCP returns binary responses up to " + f"{limit_mib} MiB; use an Appwrite SDK or REST API for larger content." + ) + tool_definition = Tool( name=tool_name, - description=docstring.short_description or "No description available", + description=description, input_schema={ "type": "object", "properties": properties, diff --git a/tests/unit/test_error_classification.py b/tests/unit/test_error_classification.py index 79b0b8d..6d657d4 100644 --- a/tests/unit/test_error_classification.py +++ b/tests/unit/test_error_classification.py @@ -4,6 +4,7 @@ from pydantic import BaseModel, ValidationError from mcp_server_appwrite.error_classification import ( + HostedBinaryResponseTooLarge, WriteConfirmationRequired, classify_tool_error, is_response_parse_error, @@ -17,6 +18,10 @@ def test_write_confirmation(self): "write_confirmation", ) + def test_hosted_binary_response_too_large(self): + error = HostedBinaryResponseTooLarge("storage_get_file_download", 1024) + self.assertEqual(classify_tool_error(error), "response_too_large") + def test_wrapped_appwrite_4xx(self): for code in (400, 401, 404, 409, 429, 499): with self.subTest(code=code): diff --git a/tests/unit/test_server.py b/tests/unit/test_server.py index 73f7454..d331d55 100644 --- a/tests/unit/test_server.py +++ b/tests/unit/test_server.py @@ -1,6 +1,7 @@ import asyncio import base64 import io +import json import os import sys import tempfile @@ -9,6 +10,7 @@ from pathlib import Path from unittest.mock import Mock, patch +import httpx import mcp.types as types from appwrite_console.enums.browser import Browser from appwrite_console.exception import AppwriteException @@ -19,6 +21,7 @@ from mcp_server_appwrite.catalog_policy import API_KEY_PROFILE, OAUTH_PROFILE from mcp_server_appwrite.error_classification import WriteConfirmationRequired from mcp_server_appwrite.server import ( + _bounded_binary_client_call, _coerce_argument, _configure_uploads, _execute_public_tool_for_transport, @@ -45,15 +48,27 @@ class _FakeResponse: - def __init__(self, *, data=b"", headers=None, url="https://example.com/pic.png"): + def __init__( + self, + *, + data=b"", + headers=None, + url="https://example.com/pic.png", + status_code=200, + reason_phrase="OK", + ): self._data = data self.headers = headers or {} self.url = url + self.status_code = status_code + self.reason_phrase = reason_phrase + self.iterated = False def raise_for_status(self): return None def iter_bytes(self): + self.iterated = True for index in range(0, len(self._data), 64): yield self._data[index : index + 64] @@ -72,6 +87,7 @@ def __exit__(self, *args): class _FakeClient: def __init__(self, response): self._response = response + self.stream_kwargs = None def __enter__(self): return self @@ -79,7 +95,8 @@ def __enter__(self): def __exit__(self, *args): return False - def stream(self, method, url): + def stream(self, method, url, **kwargs): + self.stream_kwargs = {"method": method, "url": url, **kwargs} return _FakeStream(self._response) @@ -443,6 +460,110 @@ def test_format_tool_result_returns_binary_resource(self): self.assertIsInstance(result[0], types.EmbeddedResource) self.assertEqual(result[0].resource.mime_type, "application/octet-stream") + def test_bounded_binary_call_returns_content_within_limit(self): + client = build_client_for_request( + "console", + "secret", + target_project="project-1", + organization_id="organization-1", + ) + response = _FakeResponse(data=b"plain-bytes", headers={"content-length": "11"}) + http_client = _FakeClient(response) + + with patch.object(server_module.httpx, "Client", return_value=http_client): + result = _bounded_binary_client_call( + client, + "storage_get_file_download", + "get", + "/download", + params={"token": "file-token"}, + ) + + self.assertEqual(result, b"plain-bytes") + request = http_client.stream_kwargs + self.assertEqual(request["headers"]["accept-encoding"], "identity") + self.assertEqual(request["headers"]["authorization"], "Bearer secret") + self.assertEqual(request["headers"]["x-appwrite-project"], "project-1") + self.assertEqual( + request["headers"]["x-appwrite-organization"], "organization-1" + ) + self.assertEqual(request["params"], {"token": "file-token"}) + + def test_bounded_binary_call_rejects_declared_oversize_before_reading(self): + client = build_introspection_client() + response = _FakeResponse(data=b"unread", headers={"content-length": "11"}) + + with ( + patch.object(server_module, "MAX_HOSTED_BINARY_RESPONSE_BYTES", 10), + patch.object( + server_module.httpx, "Client", return_value=_FakeClient(response) + ), + ): + with self.assertRaises( + server_module.HostedBinaryResponseTooLarge + ) as raised: + _bounded_binary_client_call( + client, "storage_get_file_download", "get", "/download" + ) + + error = json.loads(str(raised.exception))["error"] + self.assertEqual(error["code"], "hosted_response_too_large") + self.assertEqual(error["limitBytes"], 10) + self.assertEqual(error["contentLength"], 11) + + def test_bounded_binary_call_rejects_compressed_response_before_iteration(self): + client = build_introspection_client() + response = _FakeResponse( + data=b"compressed", headers={"content-encoding": "gzip"} + ) + + with patch.object( + server_module.httpx, "Client", return_value=_FakeClient(response) + ): + with self.assertRaisesRegex(ValueError, "compressed binary response"): + _bounded_binary_client_call( + client, "storage_get_file_download", "get", "/download" + ) + + self.assertFalse(response.iterated) + + def test_bounded_binary_call_wraps_httpx_transport_errors(self): + client = build_introspection_client() + failure = httpx.ConnectError("connection failed") + + with patch.object( + server_module, + "_perform_bounded_binary_client_call", + side_effect=failure, + ): + with self.assertRaises(AppwriteException) as raised: + _bounded_binary_client_call( + client, "storage_get_file_download", "get", "/download" + ) + + self.assertIs(raised.exception.__cause__, failure) + + def test_bounded_binary_call_rejects_stream_without_content_length(self): + client = build_introspection_client() + response = _FakeResponse(data=b"eleven-byte") + + with ( + patch.object(server_module, "MAX_HOSTED_BINARY_RESPONSE_BYTES", 10), + patch.object( + server_module.httpx, "Client", return_value=_FakeClient(response) + ), + ): + with self.assertRaises( + server_module.HostedBinaryResponseTooLarge + ) as raised: + _bounded_binary_client_call( + client, "storage_get_file_view", "get", "/view" + ) + + error = json.loads(str(raised.exception))["error"] + self.assertEqual(error["code"], "hosted_response_too_large") + self.assertGreater(error["observedBytes"], 10) + def test_format_appwrite_error_truncates_large_html_body(self): exc = AppwriteException("" + ("x" * 1000), 404, None) @@ -994,6 +1115,79 @@ def get_browser(self, code, width=None, height=None): self.assertEqual(captured["width"], 1) self.assertEqual(captured["height"], 1) + def test_hosted_binary_tool_uses_bounded_streaming_call(self): + tool = types.Tool( + name="storage_get_file_download", + description="Download a file.", + inputSchema={"type": "object", "properties": {}, "required": []}, + ) + manager = ToolManager() + manager.tools_registry = { + "storage_get_file_download": { + "definition": tool, + "service_name": "storage", + "method_name": "get_file_download", + "parameter_types": {}, + } + } + + class StorageService: + def __init__(self, client): + self.client = client + + def get_file_download(self) -> bytes: + return self.client.call("get", "/download", {}, {}) + + client = build_introspection_client() + with ( + patch.dict(server_module.SERVICE_CLASSES, {"storage": StorageService}), + patch.object(server_module, "resolve_client", return_value=client), + patch.object( + server_module, + "_bounded_binary_client_call", + return_value=b"bounded", + ) as bounded_call, + ): + result = execute_registered_tool(manager, tool.name, {}) + + bounded_call.assert_called_once() + self.assertIsInstance(result[0], types.EmbeddedResource) + + def test_explicit_stdio_client_keeps_sdk_binary_call(self): + tool = types.Tool( + name="storage_get_file_download", + description="Download a file.", + inputSchema={"type": "object", "properties": {}, "required": []}, + ) + manager = ToolManager() + manager.tools_registry = { + "storage_get_file_download": { + "definition": tool, + "service_name": "storage", + "method_name": "get_file_download", + "parameter_types": {}, + } + } + + class StorageService: + def __init__(self, client): + self.client = client + + def get_file_download(self) -> bytes: + return self.client.call("get", "/download", {}, {}) + + client = build_introspection_client() + client.call = Mock(return_value=b"sdk") + with ( + patch.dict(server_module.SERVICE_CLASSES, {"storage": StorageService}), + patch.object(server_module, "_bounded_binary_client_call") as bounded_call, + ): + result = execute_registered_tool(manager, tool.name, {}, client=client) + + bounded_call.assert_not_called() + client.call.assert_called_once() + self.assertIsInstance(result[0], types.EmbeddedResource) + def test_execute_registered_tool_captures_publishable_appwrite_error(self): tool = types.Tool( name="users_list", diff --git a/tests/unit/test_service.py b/tests/unit/test_service.py index 47e6ac5..bb75cde 100644 --- a/tests/unit/test_service.py +++ b/tests/unit/test_service.py @@ -51,6 +51,12 @@ def create( return {"ok": True} +class BinaryService: + def download(self) -> bytes: + """Download binary content.""" + return b"content" + + class QueryService: def list(self, queries: List[str] = [], search: str = "") -> Dict[str, Any]: """ @@ -84,6 +90,15 @@ def test_generates_enum_and_input_file_schema(self): self.assertIn("file", schema["required"]) self.assertTrue(schema["additionalProperties"] is False) + def test_documents_hosted_binary_response_limit(self): + definition = Service( + BinaryService(), "binary", binary_response_limit=25 * 1024 * 1024 + ).list_tools()["binary_download"]["definition"] + + self.assertIn( + "Hosted MCP returns binary responses up to 25 MiB", definition.description + ) + def test_documents_the_query_wire_format(self): properties = ( Service(QueryService(), "example") From 8d936e41d1dc07774b0adbe61626955ebe9c79a3 Mon Sep 17 00:00:00 2001 From: Chirag Aggarwal Date: Mon, 31 Aug 2026 14:37:20 +0100 Subject: [PATCH 2/2] fix: guard compressed upstream error bodies --- src/mcp_server_appwrite/server.py | 9 ++++++--- tests/unit/test_server.py | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/mcp_server_appwrite/server.py b/src/mcp_server_appwrite/server.py index fd0693a..c5ceb56 100644 --- a/src/mcp_server_appwrite/server.py +++ b/src/mcp_server_appwrite/server.py @@ -880,9 +880,9 @@ def _perform_bounded_binary_client_call( with http_client.stream( method, endpoint + path, headers=request_headers, params=request_params ) as response: - if response.status_code >= 400: - _raise_bounded_response_error(response) - + # Check before reading success or error bodies: HTTPX decodes + # ``iter_bytes()`` chunks, so either path could otherwise inflate a + # compressed response beyond the limit before we can count it. content_encoding = response.headers.get("content-encoding", "identity") if content_encoding.lower().strip() not in {"", "identity"}: raise ValueError( @@ -890,6 +890,9 @@ def _perform_bounded_binary_client_call( "Use an Appwrite SDK or REST API for this content." ) + if response.status_code >= 400: + _raise_bounded_response_error(response) + warning = response.headers.get("x-appwrite-warning") if warning: for item in warning.split(";"): diff --git a/tests/unit/test_server.py b/tests/unit/test_server.py index d331d55..1689bcc 100644 --- a/tests/unit/test_server.py +++ b/tests/unit/test_server.py @@ -527,6 +527,25 @@ def test_bounded_binary_call_rejects_compressed_response_before_iteration(self): self.assertFalse(response.iterated) + def test_bounded_binary_call_rejects_compressed_error_before_iteration(self): + client = build_introspection_client() + response = _FakeResponse( + data=b"compressed error", + headers={"content-encoding": "gzip"}, + status_code=502, + reason_phrase="Bad Gateway", + ) + + with patch.object( + server_module.httpx, "Client", return_value=_FakeClient(response) + ): + with self.assertRaisesRegex(ValueError, "compressed binary response"): + _bounded_binary_client_call( + client, "storage_get_file_download", "get", "/download" + ) + + self.assertFalse(response.iterated) + def test_bounded_binary_call_wraps_httpx_transport_errors(self): client = build_introspection_client() failure = httpx.ConnectError("connection failed")