From 7cd768bb0ef48ec2eb2984adbea57943d33817f1 Mon Sep 17 00:00:00 2001 From: nolanefe Date: Wed, 12 Aug 2026 20:02:57 +0300 Subject: [PATCH 1/2] feat: add public REST-backed MCP tools --- .gitignore | 1 + README.md | 2 + application/mcp/__init__.py | 5 + application/mcp/__main__.py | 22 + application/mcp/catalog.py | 96 +++++ application/mcp/openapi_loader.py | 152 +++++++ application/mcp/rest_client.py | 318 +++++++++++++++ application/mcp/server.py | 139 +++++++ application/tests/mcp_public_tools_test.py | 449 +++++++++++++++++++++ docs/api/mcp.md | 106 +++++ requirements-dev.txt | 3 + 11 files changed, 1293 insertions(+) create mode 100644 application/mcp/__init__.py create mode 100644 application/mcp/__main__.py create mode 100644 application/mcp/catalog.py create mode 100644 application/mcp/openapi_loader.py create mode 100644 application/mcp/rest_client.py create mode 100644 application/mcp/server.py create mode 100644 application/tests/mcp_public_tools_test.py create mode 100644 docs/api/mcp.md diff --git a/.gitignore b/.gitignore index 26b6ceb61..45d523728 100644 --- a/.gitignore +++ b/.gitignore @@ -68,6 +68,7 @@ standards_cache.sqlite *.md !AGENTS.md !docs/faq.md +!docs/api/mcp.md !docs/Mid_eval_blog_gsoc2026/module_B_mideval_blog.md ### Dev DBDumps diff --git a/README.md b/README.md index d61916f0a..0bb430f32 100644 --- a/README.md +++ b/README.md @@ -288,6 +288,8 @@ http://127.0.0.1:5000 See [the myOpenCRE user guide](docs/my-opencre-user-guide.md) on using the OpenCRE API to, for example, add your own security guidelines and standards. +For a local stdio MCP server over public REST reads, see [docs/api/mcp.md](docs/api/mcp.md). + ## Docker building and running You can build the production or the development docker images with: diff --git a/application/mcp/__init__.py b/application/mcp/__init__.py new file mode 100644 index 000000000..28f3086ae --- /dev/null +++ b/application/mcp/__init__.py @@ -0,0 +1,5 @@ +"""Local stdio MCP server exposing public OpenCRE REST reads (issue #1003 v1).""" + +from application.mcp.catalog import PUBLIC_TOOLS, get_tool, list_tool_names + +__all__ = ["PUBLIC_TOOLS", "get_tool", "list_tool_names"] diff --git a/application/mcp/__main__.py b/application/mcp/__main__.py new file mode 100644 index 000000000..7667bd7e7 --- /dev/null +++ b/application/mcp/__main__.py @@ -0,0 +1,22 @@ +"""Entry point: python -m application.mcp""" + +from __future__ import annotations + +import asyncio +import logging +import sys + + +def main() -> None: + logging.basicConfig( + level=logging.INFO, + stream=sys.stderr, + format="%(levelname)s %(name)s: %(message)s", + ) + from application.mcp.server import run_stdio + + asyncio.run(run_stdio()) + + +if __name__ == "__main__": + main() diff --git a/application/mcp/catalog.py b/application/mcp/catalog.py new file mode 100644 index 000000000..c91981b0a --- /dev/null +++ b/application/mcp/catalog.py @@ -0,0 +1,96 @@ +"""Static security allowlist for OpenCRE MCP v1 public-read tools. + +Exposure is determined only by this allowlist — never by enumerating OpenAPI. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from typing import Dict, List, Tuple + + +@dataclass(frozen=True) +class ToolSpec: + """One MCP tool bound to a fixed public REST GET operation.""" + + name: str + method: str + path_template: str + summary: str + + @property + def openapi_identity(self) -> Tuple[str, str]: + """(HTTP method lowercased, OpenAPI path) used to resolve schemas.""" + return (self.method.lower(), self.path_template) + + +# Exact v1 surface approved for the first MCP PR (public JSON reads only). +PUBLIC_TOOLS: Tuple[ToolSpec, ...] = ( + ToolSpec( + name="get_cre_by_id", + method="GET", + path_template="/rest/v1/id/{creid}", + summary="Get a CRE by ID", + ), + ToolSpec( + name="get_cre_by_name", + method="GET", + path_template="/rest/v1/name/{crename}", + summary="Get a CRE by name", + ), + ToolSpec( + name="get_node", + method="GET", + path_template="/rest/v1/{ntype}/{name}", + summary="Get nodes by type and name", + ), + ToolSpec( + name="get_documents_by_tag", + method="GET", + path_template="/rest/v1/tags", + summary="Get documents by tag", + ), + ToolSpec( + name="text_search", + method="GET", + path_template="/rest/v1/text_search", + summary="Text search", + ), + ToolSpec( + name="list_root_cres", + method="GET", + path_template="/rest/v1/root_cres", + summary="Get root CREs", + ), + ToolSpec( + name="list_all_cres", + method="GET", + path_template="/rest/v1/all_cres", + summary="List all CREs (paginated)", + ), + ToolSpec( + name="list_standards", + method="GET", + path_template="/rest/v1/standards", + summary="List standards", + ), + ToolSpec( + name="list_ga_standards", + method="GET", + path_template="/rest/v1/ga_standards", + summary="Standards eligible for gap analysis", + ), +) + +_TOOLS_BY_NAME: Dict[str, ToolSpec] = {tool.name: tool for tool in PUBLIC_TOOLS} + + +def list_tool_names() -> List[str]: + return [tool.name for tool in PUBLIC_TOOLS] + + +def get_tool(name: str) -> ToolSpec: + try: + return _TOOLS_BY_NAME[name] + except KeyError as exc: + raise KeyError(f"Unknown MCP tool: {name}") from exc diff --git a/application/mcp/openapi_loader.py b/application/mcp/openapi_loader.py new file mode 100644 index 000000000..8fbd272f4 --- /dev/null +++ b/application/mcp/openapi_loader.py @@ -0,0 +1,152 @@ +"""Load MCP tool input schemas from the committed OpenAPI document. + +OpenAPI is the source of truth for selected allowlisted operations' parameters. +This module does not enumerate OpenAPI to decide which tools exist. +""" + +from __future__ import annotations + +import copy +import os +from functools import lru_cache +from typing import Any, Dict, List, Optional, Set + +import yaml + +from application.mcp.catalog import PUBLIC_TOOLS, ToolSpec, get_tool + +# Response-format switchers that can leave JSON; omit from MCP v1 inputs. +_JSON_UNSAFE_QUERY_PARAMS: Set[str] = {"format"} + +REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) +DEFAULT_OPENAPI_PATH = os.path.join(REPO_ROOT, "docs", "api", "openapi.yaml") + + +class OpenAPILookupError(LookupError): + """Raised when an allowlisted method/path is missing or unusable in OpenAPI.""" + + +def _load_spec(path: str) -> Dict[str, Any]: + with open(path, "r", encoding="utf-8") as handle: + data = yaml.safe_load(handle) + if not isinstance(data, dict) or "paths" not in data: + raise OpenAPILookupError(f"Invalid OpenAPI document: {path}") + return data + + +def _resolve_ref(spec: Dict[str, Any], node: Any) -> Any: + """Resolve local #/components/... refs one level deep as needed.""" + if not isinstance(node, dict): + return node + if "$ref" not in node: + return {key: _resolve_ref(spec, value) for key, value in node.items()} + ref = node["$ref"] + if not isinstance(ref, str) or not ref.startswith("#/"): + raise OpenAPILookupError(f"Unsupported OpenAPI $ref: {ref}") + current: Any = spec + for part in ref[2:].split("/"): + if not isinstance(current, dict) or part not in current: + raise OpenAPILookupError(f"Unresolved OpenAPI $ref: {ref}") + current = current[part] + # Merge sibling keywords onto the resolved target (OpenAPI 3.0.3). + resolved = copy.deepcopy(current) + if isinstance(resolved, dict): + for key, value in node.items(): + if key == "$ref": + continue + resolved[key] = value + return _resolve_ref(spec, resolved) + return resolved + + +def _parameter_to_json_schema( + spec: Dict[str, Any], param: Dict[str, Any] +) -> Dict[str, Any]: + schema = param.get("schema") + if schema is None and "content" in param: + raise OpenAPILookupError( + f"Unsupported content-based parameter '{param.get('name')}'" + ) + if schema is None: + schema = {"type": "string"} + resolved = _resolve_ref(spec, schema) + if not isinstance(resolved, dict): + raise OpenAPILookupError( + f"Parameter '{param.get('name')}' schema must be an object" + ) + out = copy.deepcopy(resolved) + description = param.get("description") + if description and "description" not in out: + out["description"] = description + return out + + +def operation_input_schema( + tool: ToolSpec, *, spec: Optional[Dict[str, Any]] = None +) -> Dict[str, Any]: + """Build a JSON Schema object for MCP tool arguments from OpenAPI parameters.""" + document = spec if spec is not None else load_openapi_spec() + method, path = tool.openapi_identity + paths = document.get("paths") or {} + path_item = paths.get(path) + if not isinstance(path_item, dict): + raise OpenAPILookupError( + f"Allowlisted OpenAPI path missing: {method.upper()} {path}" + ) + operation = path_item.get(method) + if not isinstance(operation, dict): + raise OpenAPILookupError( + f"Allowlisted OpenAPI operation missing: {method.upper()} {path}" + ) + + properties: Dict[str, Any] = {} + required: List[str] = [] + for raw_param in operation.get("parameters") or []: + param = _resolve_ref(document, raw_param) + if not isinstance(param, dict): + raise OpenAPILookupError(f"Invalid parameter on {method.upper()} {path}") + location = param.get("in") + name = param.get("name") + if not name or location not in ("path", "query"): + continue + if location == "query" and name in _JSON_UNSAFE_QUERY_PARAMS: + continue + properties[name] = _parameter_to_json_schema(document, param) + if param.get("required"): + required.append(name) + + schema: Dict[str, Any] = { + "type": "object", + "properties": properties, + "additionalProperties": False, + } + if required: + schema["required"] = required + return schema + + +@lru_cache(maxsize=1) +def load_openapi_spec(path: str = DEFAULT_OPENAPI_PATH) -> Dict[str, Any]: + return _load_spec(path) + + +def clear_openapi_cache() -> None: + load_openapi_spec.cache_clear() + + +def input_schema_for_tool(name: str) -> Dict[str, Any]: + return operation_input_schema(get_tool(name)) + + +def all_tool_input_schemas() -> Dict[str, Dict[str, Any]]: + """Resolve input schemas for every allowlisted tool (fails if OpenAPI drifts).""" + spec = load_openapi_spec() + return {tool.name: operation_input_schema(tool, spec=spec) for tool in PUBLIC_TOOLS} + + +def openapi_has_operation(method: str, path: str) -> bool: + spec = load_openapi_spec() + path_item = (spec.get("paths") or {}).get(path) + if not isinstance(path_item, dict): + return False + return isinstance(path_item.get(method.lower()), dict) diff --git a/application/mcp/rest_client.py b/application/mcp/rest_client.py new file mode 100644 index 000000000..38550de0c --- /dev/null +++ b/application/mcp/rest_client.py @@ -0,0 +1,318 @@ +"""HTTP adapter from MCP tools to the OpenCRE public REST API. + +REST remains the execution and security boundary. Callers cannot choose the +base URL, HTTP method, or arbitrary paths. +""" + +from __future__ import annotations + +import json +import os +import re +from dataclasses import dataclass +from typing import Any, Dict, Mapping, MutableMapping, Optional, Sequence, Set +from urllib.parse import quote, urljoin + +import requests +from jsonschema import Draft7Validator +from jsonschema.exceptions import ValidationError + +from application.defs import cre_defs as defs +from application.mcp.catalog import ToolSpec, get_tool +from application.mcp.openapi_loader import operation_input_schema + +DEFAULT_BASE_URL = "http://127.0.0.1:5000" +DEFAULT_TIMEOUT_SECONDS = 30.0 + +# Path params must be single URL segments — no separators that alter routing. +_UNSAFE_PATH_PARAM = re.compile(r"[/?#]") + +# First-path segments under /rest/v1 that are static routes, not Credoctypes. +# Blocked so get_node cannot collide with MyOpenCRE / auth / other surfaces. +_RESERVED_NTYPES: Set[str] = { + "user", + "id", + "name", + "tags", + "standards", + "ga_standards", + "text_search", + "root_cres", + "all_cres", + "map_analysis", + "map_analysis_weak_links", + "ma_job_results", + "health", + "config", + "completion", + "login", + "logout", + "callback", + "cre_csv", + "cre_csv_import", + "openapi.yaml", + "deeplink", +} + + +class RestClientError(Exception): + """Base error for MCP REST adapter failures.""" + + +class RestRequestError(RestClientError): + """Invalid tool arguments or unsafe path construction.""" + + +class RestResponseError(RestClientError): + """Non-success HTTP response from OpenCRE REST.""" + + def __init__(self, status_code: int, message: str, body: Any = None) -> None: + super().__init__(message) + self.status_code = status_code + self.body = body + + +@dataclass(frozen=True) +class RestResult: + status_code: int + data: Any + text: str + + +def normalize_base_url(base_url: Optional[str] = None) -> str: + raw = ( + base_url if base_url is not None else os.environ.get("OPENCRE_BASE_URL") + ) or (DEFAULT_BASE_URL) + return raw.rstrip("/") + + +def _new_session() -> requests.Session: + """Session that never keeps cookies or env/netrc credentials (v1 public only).""" + session = requests.Session() + # Disable Requests env/netrc credential pickup (trust_env defaults to True). + session.trust_env = False + session.cookies.clear() + return session + + +class RestClient: + """Execute allowlisted GET templates against OPENCRE_BASE_URL.""" + + def __init__( + self, + *, + base_url: Optional[str] = None, + session: Optional[Any] = None, + timeout: float = DEFAULT_TIMEOUT_SECONDS, + ) -> None: + # base_url is constructor/env only — never a tool argument. + self._base_url = normalize_base_url(base_url) + self._session = session if session is not None else _new_session() + self._timeout = timeout + + @property + def base_url(self) -> str: + return self._base_url + + def call_tool( + self, tool_name: str, arguments: Optional[Mapping[str, Any]] = None + ) -> RestResult: + try: + tool = get_tool(tool_name) + except KeyError as exc: + raise RestRequestError(str(exc)) from exc + return self.call_spec(tool, arguments or {}) + + def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult: + if tool.method.upper() != "GET": + raise RestRequestError(f"Only GET is supported (got {tool.method})") + + schema = operation_input_schema(tool) + path_param_names = _path_param_names(tool.path_template) + allowed = set((schema.get("properties") or {}).keys()) + required = set(schema.get("required") or []) + + args = dict(arguments) + unknown = sorted(set(args) - allowed) + if unknown: + raise RestRequestError( + f"Unexpected argument(s) for {tool.name}: {', '.join(unknown)}" + ) + missing = sorted(required - set(args)) + if missing: + raise RestRequestError( + f"Missing required argument(s) for {tool.name}: {', '.join(missing)}" + ) + # Enforce the exact OpenAPI-derived JSON Schema (types, enums, etc.). + _validate_against_input_schema(tool.name, schema, args) + + path_params: Dict[str, str] = {} + query_params: Dict[str, Any] = {} + for key, value in args.items(): + if key in path_param_names: + path_params[key] = _sanitize_path_value(key, value) + else: + query_params[key] = value + + missing_path = sorted(set(path_param_names) - set(path_params)) + if missing_path: + raise RestRequestError( + f"Missing path parameter(s) for {tool.name}: {', '.join(missing_path)}" + ) + + _validate_node_type_param(path_params) + path = _render_path(tool.path_template, path_params) + url = urljoin(self._base_url + "/", path.lstrip("/")) + response = self._session.get( + url, + params=_encode_query(query_params), + timeout=self._timeout, + allow_redirects=False, + ) + # Never retain cookies across tool calls (no credential forwarding). + if hasattr(self._session, "cookies"): + try: + self._session.cookies.clear() + except Exception: + pass + return _parse_response(response) + + +def flask_test_session(flask_client: Any) -> "_FlaskTestSession": + """Adapt Flask's test_client to the requests Session.get interface.""" + return _FlaskTestSession(flask_client) + + +class _FlaskTestResponse: + def __init__(self, response: Any) -> None: + self.status_code = int(response.status_code) + raw = response.get_data(as_text=True) + self.text = ( + raw if isinstance(raw, str) else raw.decode("utf-8", errors="replace") + ) + self.headers = getattr(response, "headers", {}) + + def json(self) -> Any: + return json.loads(self.text) + + +class _FlaskTestSession: + def __init__(self, client: Any) -> None: + self._client = client + self.cookies = requests.cookies.RequestsCookieJar() + + def get( + self, + url: str, + params: Optional[Any] = None, + timeout: Optional[float] = None, + allow_redirects: bool = False, + **_kwargs: Any, + ) -> _FlaskTestResponse: + del timeout, allow_redirects # parity with requests; unused by test client + from urllib.parse import urlparse + + parsed = urlparse(url) + path = parsed.path or "/" + response = self._client.get(path, query_string=params) + return _FlaskTestResponse(response) + + +def _validate_against_input_schema( + tool_name: str, schema: Mapping[str, Any], arguments: Mapping[str, Any] +) -> None: + """Validate tool args with Draft7Validator (OpenAPI 3.0.3 JSON Schema subset).""" + try: + Draft7Validator(schema).validate(arguments) + except ValidationError as exc: + path = ".".join(str(part) for part in exc.absolute_path) + detail = f"{path}: {exc.message}" if path else exc.message + raise RestRequestError(f"Invalid arguments for {tool_name}: {detail}") from exc + + +def _path_param_names(template: str) -> Sequence[str]: + return re.findall(r"\{([^}]+)\}", template) + + +def _sanitize_path_value(name: str, value: Any) -> str: + if value is None or isinstance(value, (dict, list, bool)): + raise RestRequestError(f"Invalid path parameter '{name}'") + text = str(value).strip() + if not text: + raise RestRequestError(f"Path parameter '{name}' must be non-empty") + if _UNSAFE_PATH_PARAM.search(text) or ".." in text or text.startswith("//"): + raise RestRequestError(f"Path parameter '{name}' contains unsafe characters") + return text + + +def _validate_node_type_param(path_params: Mapping[str, str]) -> None: + """Prevent get_node path collisions with static /rest/v1 routes (e.g. user/resources).""" + if "ntype" not in path_params: + return + ntype = path_params["ntype"] + lowered = ntype.lower() + if lowered in _RESERVED_NTYPES: + raise RestRequestError( + f"Path parameter 'ntype' value '{ntype}' is reserved and not a node type" + ) + allowed = {t.value.lower() for t in defs.Credoctypes} + # Match Flask find_node_by_name: case-insensitive Credoctypes only. + if lowered not in allowed: + raise RestRequestError( + f"Path parameter 'ntype' must be one of: " + f"{', '.join(sorted(t.value for t in defs.Credoctypes))}" + ) + + +def _render_path(template: str, path_params: Mapping[str, str]) -> str: + path = template + for name, value in path_params.items(): + token = "{" + name + "}" + if token not in path: + raise RestRequestError(f"Unknown path parameter '{name}' for template") + path = path.replace(token, quote(value, safe="")) + if "{" in path or "}" in path: + raise RestRequestError("Unresolved path parameters in template") + if ".." in path.split("/"): + raise RestRequestError("Refusing path traversal in rendered route") + return path + + +def _encode_query(query_params: Mapping[str, Any]) -> MutableMapping[str, Any]: + """Preserve list query params for Flask's getlist / OpenAPI explode style.""" + encoded: Dict[str, Any] = {} + for key, value in query_params.items(): + if value is None: + continue + if isinstance(value, (list, tuple)): + encoded[key] = [str(item) for item in value] + elif isinstance(value, bool): + encoded[key] = "true" if value else "false" + else: + encoded[key] = value + return encoded + + +def _parse_response(response: Any) -> RestResult: + status = int(getattr(response, "status_code", 0)) + text = getattr(response, "text", "") or "" + data: Any + try: + data = response.json() + except Exception: + data = text + + if status < 200 or status >= 300: + if isinstance(data, dict): + message = ( + data.get("message") + or data.get("error") + or data.get("description") + or text + or f"HTTP {status}" + ) + else: + message = text or f"HTTP {status}" + raise RestResponseError(status, f"REST {status}: {message}", body=data) + + return RestResult(status_code=status, data=data, text=text) diff --git a/application/mcp/server.py b/application/mcp/server.py new file mode 100644 index 000000000..967186812 --- /dev/null +++ b/application/mcp/server.py @@ -0,0 +1,139 @@ +"""Low-level MCP stdio server for OpenCRE public REST tools.""" + +from __future__ import annotations + +import json +import logging +from typing import Any, Dict, Optional + +import mcp.types as types +from mcp.server.lowlevel import Server +from mcp.server.stdio import stdio_server + +from application.mcp.catalog import PUBLIC_TOOLS, get_tool, list_tool_names +from application.mcp.openapi_loader import ( + all_tool_input_schemas, + operation_input_schema, +) +from application.mcp.rest_client import ( + RestClient, + RestClientError, + RestRequestError, + RestResponseError, +) + +logger = logging.getLogger(__name__) + +SERVER_NAME = "opencre" +SERVER_VERSION = "0.1.0" +SERVER_INSTRUCTIONS = ( + "OpenCRE MCP v1 exposes public JSON REST reads only. " + "Authenticated MyOpenCRE, chat, and admin tools are out of scope." +) + + +def build_server(rest_client: Optional[RestClient] = None) -> Server[Any]: + """Create an MCP Server with OpenAPI-derived tool schemas and REST dispatch.""" + client = rest_client or RestClient() + # Resolve schemas once at startup so OpenAPI drift fails before serving. + schemas = all_tool_input_schemas() + + async def on_list_tools( + ctx: Any, params: types.PaginatedRequestParams | None + ) -> types.ListToolsResult: + del ctx, params + tools = [ + types.Tool( + name=tool.name, + description=tool.summary, + input_schema=schemas[tool.name], + annotations=types.ToolAnnotations( + read_only_hint=True, + destructive_hint=False, + idempotent_hint=True, + open_world_hint=True, + ), + ) + for tool in PUBLIC_TOOLS + ] + return types.ListToolsResult(tools=tools) + + async def on_call_tool( + ctx: Any, params: types.CallToolRequestParams + ) -> types.CallToolResult: + del ctx + name = params.name + arguments: Dict[str, Any] = dict(params.arguments or {}) + try: + if name not in list_tool_names(): + raise RestRequestError(f"Unknown MCP tool: {name}") + # Ensure allowlist entry still matches OpenAPI before calling REST. + get_tool(name) + operation_input_schema(get_tool(name)) + result = client.call_tool(name, arguments) + payload = json.dumps(result.data, ensure_ascii=False, default=str) + structured: Dict[str, Any] + if isinstance(result.data, dict): + structured = result.data + else: + structured = {"result": result.data} + return types.CallToolResult( + content=[types.TextContent(type="text", text=payload)], + structured_content=structured, + is_error=False, + ) + except RestResponseError as exc: + logger.info("REST error for tool %s: %s", name, exc) + return types.CallToolResult( + content=[types.TextContent(type="text", text=str(exc))], + is_error=True, + ) + except RestRequestError as exc: + logger.info("Bad tool request for %s: %s", name, exc) + return types.CallToolResult( + content=[types.TextContent(type="text", text=str(exc))], + is_error=True, + ) + except RestClientError as exc: + logger.exception("REST client failure for tool %s", name) + return types.CallToolResult( + content=[types.TextContent(type="text", text=str(exc))], + is_error=True, + ) + except Exception: # pragma: no cover - defensive + logger.exception("Unexpected MCP tool failure for %s", name) + return types.CallToolResult( + content=[ + types.TextContent(type="text", text="Internal MCP tool error.") + ], + is_error=True, + ) + + return Server( + SERVER_NAME, + version=SERVER_VERSION, + instructions=SERVER_INSTRUCTIONS, + on_list_tools=on_list_tools, + on_call_tool=on_call_tool, + ) + + +async def run_stdio(rest_client: Optional[RestClient] = None) -> None: + server = build_server(rest_client=rest_client) + async with stdio_server() as (read_stream, write_stream): + await server.run( + read_stream, + write_stream, + server.create_initialization_options(), + ) + + +def dispatch_tool( + tool_name: str, + arguments: Optional[Dict[str, Any]] = None, + *, + rest_client: Optional[RestClient] = None, +) -> Any: + """Synchronous helper used by tests: catalog → REST, return decoded JSON.""" + client = rest_client or RestClient() + return client.call_tool(tool_name, arguments or {}).data diff --git a/application/tests/mcp_public_tools_test.py b/application/tests/mcp_public_tools_test.py new file mode 100644 index 000000000..2ac4d3298 --- /dev/null +++ b/application/tests/mcp_public_tools_test.py @@ -0,0 +1,449 @@ +"""Tests for OpenCRE MCP v1 public REST tools (issue #1003).""" + +from __future__ import annotations + +import json +import os +import unittest +from typing import Any, Dict +from unittest.mock import patch + +import networkx as nx + +from application import create_app, sqla # type: ignore +from application.database import db +from application.defs import cre_defs as defs +from application.mcp import catalog +from application.mcp.catalog import PUBLIC_TOOLS, get_tool, list_tool_names +from application.mcp.openapi_loader import ( + OpenAPILookupError, + all_tool_input_schemas, + clear_openapi_cache, + input_schema_for_tool, + load_openapi_spec, + operation_input_schema, +) +from application.mcp.rest_client import ( + RestClient, + RestRequestError, + RestResponseError, + flask_test_session, +) +from application.mcp.server import build_server, dispatch_tool + +import mcp.types as types + + +APPROVED_TOOLS = [ + "get_cre_by_id", + "get_cre_by_name", + "get_node", + "get_documents_by_tag", + "text_search", + "list_root_cres", + "list_all_cres", + "list_standards", + "list_ga_standards", +] + + +class McpPublicToolsTest(unittest.TestCase): + def setUp(self) -> None: + self.app = create_app(mode="test") + self.app_context = self.app.app_context() + self.app_context.push() + os.environ["INSECURE_REQUESTS"] = "True" + sqla.create_all() + self.collection = db.Node_collection().with_graph() + self.collection.graph.with_graph(graph=nx.DiGraph(), graph_data=[]) + clear_openapi_cache() + + def tearDown(self) -> None: + sqla.session.remove() + sqla.drop_all() + self.app_context.pop() + clear_openapi_cache() + + def _rest(self) -> RestClient: + return RestClient(session=flask_test_session(self.app.test_client())) + + def _seed_cre_and_node(self) -> Dict[str, Any]: + cre = defs.CRE(id="111-111", description="CA", name="CA", tags=["ta"]) + node = defs.Standard( + name="ASVS", + section="1.1", + subsection="", + sectionID="1.1", + hyperlink="https://example.com/asvs", + ) + self.collection.add_cre(cre) + self.collection.add_node(node) + return {"cre": cre, "node": node} + + # --- A. Catalog / security boundary --- + + def test_catalog_exposes_exactly_nine_approved_tools(self) -> None: + names = list_tool_names() + self.assertEqual(names, APPROVED_TOOLS) + self.assertEqual(len(PUBLIC_TOOLS), 9) + for tool in PUBLIC_TOOLS: + self.assertEqual(tool.method.upper(), "GET") + + def test_catalog_excludes_auth_admin_writes_and_heavy_reads(self) -> None: + names = set(list_tool_names()) + forbidden = { + "get_user_resources", + "put_user_resources", + "user_resources", + "completion", + "map_analysis", + "map_analysis_weak_links", + "get_ma_job_results", + "fetch_job", + "get_cre_csv", + "get_config", + "health", + "deeplink", + "admin_import_runs", + } + self.assertTrue(names.isdisjoint(forbidden)) + for tool in PUBLIC_TOOLS: + self.assertNotIn("/user/resources", tool.path_template) + self.assertNotIn("completion", tool.path_template) + self.assertNotIn("/admin/", tool.path_template) + self.assertNotIn("map_analysis", tool.path_template) + self.assertNotIn("cre_csv", tool.path_template) + self.assertNotIn("health", tool.path_template) + self.assertNotIn("deeplink", tool.path_template) + self.assertNotIn("section/", tool.path_template) + self.assertNotIn("sectionid/", tool.path_template) + + def test_unknown_tool_rejected(self) -> None: + with self.assertRaises(KeyError): + get_tool("not_a_real_tool") + with self.assertRaises(RestRequestError): + self._rest().call_tool("not_a_real_tool", {}) + + def test_arbitrary_path_and_base_url_impossible(self) -> None: + client = self._rest() + with self.assertRaises(RestRequestError): + client.call_tool( + "get_cre_by_id", + {"creid": "../admin", "path": "/rest/v1/user/resources"}, + ) + with self.assertRaises(RestRequestError): + client.call_tool( + "get_cre_by_id", {"creid": "111-111", "base_url": "http://evil"} + ) + # Path traversal / separator injection + with self.assertRaises(RestRequestError): + client.call_tool("get_cre_by_id", {"creid": "a/b"}) + with self.assertRaises(RestRequestError): + client.call_tool("get_node", {"ntype": "Standard", "name": "x?y=1"}) + + def test_get_node_rejects_user_resources_collision(self) -> None: + """ntype=user/name=resources must not reach MyOpenCRE REST.""" + client = self._rest() + with self.assertRaises(RestRequestError): + client.call_tool("get_node", {"ntype": "user", "name": "resources"}) + with self.assertRaises(RestRequestError): + client.call_tool("get_node", {"ntype": "User", "name": "resources"}) + with self.assertRaises(RestRequestError): + client.call_tool("get_node", {"ntype": "completion", "name": "x"}) + + def test_format_query_param_rejected(self) -> None: + client = self._rest() + with self.assertRaises(RestRequestError): + client.call_tool("get_cre_by_id", {"creid": "111-111", "format": "csv"}) + with self.assertRaises(RestRequestError): + client.call_tool("text_search", {"text": "x", "format": "md"}) + + # --- B. OpenAPI source of truth --- + + def test_every_catalog_entry_resolves_openapi_operation(self) -> None: + spec = load_openapi_spec() + for tool in PUBLIC_TOOLS: + method, path = tool.openapi_identity + self.assertIn(path, spec["paths"]) + self.assertIn(method, spec["paths"][path]) + schema = operation_input_schema(tool, spec=spec) + self.assertEqual(schema["type"], "object") + self.assertFalse(schema.get("additionalProperties", True)) + + def test_input_schemas_reflect_openapi_parameters_without_format(self) -> None: + cre_schema = input_schema_for_tool("get_cre_by_id") + self.assertIn("creid", cre_schema["required"]) + self.assertIn("creid", cre_schema["properties"]) + self.assertIn("source", cre_schema["properties"]) + self.assertIn("include_only", cre_schema["properties"]) + self.assertNotIn("format", cre_schema["properties"]) + + tag_schema = input_schema_for_tool("get_documents_by_tag") + self.assertIn("tag", tag_schema["required"]) + self.assertEqual(tag_schema["properties"]["tag"]["type"], "array") + self.assertNotIn("format", tag_schema["properties"]) + + node_schema = input_schema_for_tool("get_node") + for key in ("ntype", "name"): + self.assertIn(key, node_schema["required"]) + self.assertIn("section", node_schema["properties"]) + self.assertNotIn("format", node_schema["properties"]) + + standards_schema = input_schema_for_tool("list_standards") + self.assertIn("all", standards_schema["properties"]) + self.assertEqual(standards_schema["properties"]["all"]["type"], "boolean") + + emptyish = input_schema_for_tool("list_ga_standards") + self.assertEqual(emptyish.get("properties"), {}) + + def test_missing_allowlisted_openapi_path_fails_clearly(self) -> None: + fake = catalog.ToolSpec( + name="ghost", + method="GET", + path_template="/rest/v1/does-not-exist", + summary="ghost", + ) + with self.assertRaises(OpenAPILookupError): + operation_input_schema(fake) + + def test_non_allowlisted_openapi_get_is_not_an_mcp_tool(self) -> None: + spec = load_openapi_spec() + self.assertIn("/rest/v1/map_analysis", spec["paths"]) + self.assertIn("/rest/v1/user/resources", spec["paths"]) + self.assertIn("/rest/v1/health", spec["paths"]) + names = set(list_tool_names()) + self.assertNotIn("map_analysis", names) + self.assertEqual(len(all_tool_input_schemas()), 9) + + # --- C. REST parity --- + + def test_parity_get_cre_by_id(self) -> None: + seeded = self._seed_cre_and_node() + cre = seeded["cre"] + with self.app.test_client() as flask_client: + rest = flask_client.get(f"/rest/v1/id/{cre.id}") + mcp_data = dispatch_tool( + "get_cre_by_id", + {"creid": cre.id}, + rest_client=RestClient(session=flask_test_session(flask_client)), + ) + self.assertEqual(rest.status_code, 200) + self.assertEqual(mcp_data, rest.get_json()) + + def test_parity_get_node(self) -> None: + seeded = self._seed_cre_and_node() + node = seeded["node"] + with self.app.test_client() as flask_client: + rest = flask_client.get(f"/rest/v1/Standard/{node.name}") + mcp_data = dispatch_tool( + "get_node", + {"ntype": "Standard", "name": node.name}, + rest_client=RestClient(session=flask_test_session(flask_client)), + ) + self.assertEqual(rest.status_code, 200) + self.assertEqual(mcp_data, rest.get_json()) + + def test_parity_text_search(self) -> None: + self._seed_cre_and_node() + with self.app.test_client() as flask_client: + rest = flask_client.get("/rest/v1/text_search?text=CA") + mcp_data = dispatch_tool( + "text_search", + {"text": "CA"}, + rest_client=RestClient(session=flask_test_session(flask_client)), + ) + self.assertEqual(rest.status_code, 200) + self.assertEqual(mcp_data, rest.get_json()) + + def test_parity_list_root_cres_and_all_cres(self) -> None: + cre = defs.CRE(id="222-222", description="Root", name="RootCRE") + self.collection.add_cre(cre) + with self.app.test_client() as flask_client: + client = RestClient(session=flask_test_session(flask_client)) + root_rest = flask_client.get("/rest/v1/root_cres") + root_mcp = dispatch_tool("list_root_cres", {}, rest_client=client) + all_rest = flask_client.get("/rest/v1/all_cres") + all_mcp = dispatch_tool("list_all_cres", {}, rest_client=client) + self.assertEqual(root_rest.status_code, 200) + self.assertEqual(root_mcp, root_rest.get_json()) + self.assertEqual(all_rest.status_code, 200) + self.assertEqual(all_mcp, all_rest.get_json()) + + def test_parity_list_standards(self) -> None: + self._seed_cre_and_node() + with self.app.test_client() as flask_client: + rest = flask_client.get("/rest/v1/standards") + mcp_data = dispatch_tool( + "list_standards", + {}, + rest_client=RestClient(session=flask_test_session(flask_client)), + ) + self.assertEqual(rest.status_code, 200) + self.assertEqual(mcp_data, rest.get_json()) + + def test_parity_remaining_tools(self) -> None: + cre = defs.CRE( + id="333-333", description="Tagged", name="Tagged", tags=["alpha"] + ) + self.collection.add_cre(cre) + with self.app.test_client() as flask_client: + client = RestClient(session=flask_test_session(flask_client)) + by_name_rest = flask_client.get("/rest/v1/name/Tagged") + by_name_mcp = dispatch_tool( + "get_cre_by_name", {"crename": "Tagged"}, rest_client=client + ) + tags_rest = flask_client.get("/rest/v1/tags?tag=alpha") + tags_mcp = dispatch_tool( + "get_documents_by_tag", {"tag": ["alpha"]}, rest_client=client + ) + ga_rest = flask_client.get("/rest/v1/ga_standards") + ga_mcp = dispatch_tool("list_ga_standards", {}, rest_client=client) + self.assertEqual(by_name_mcp, by_name_rest.get_json()) + self.assertEqual(tags_mcp, tags_rest.get_json()) + self.assertEqual(ga_mcp, ga_rest.get_json()) + + # --- D. Error behavior --- + + def test_rest_404_is_tool_failure_not_empty_success(self) -> None: + client = self._rest() + with self.assertRaises(RestResponseError) as ctx: + client.call_tool("get_cre_by_id", {"creid": "999-999"}) + self.assertEqual(ctx.exception.status_code, 404) + + def test_missing_required_input_fails(self) -> None: + client = self._rest() + with self.assertRaises(RestRequestError): + client.call_tool("text_search", {}) + with self.assertRaises(RestRequestError): + client.call_tool("get_documents_by_tag", {}) + with self.assertRaises(RestRequestError): + client.call_tool("get_node", {"ntype": "Standard"}) + + def test_default_http_session_disables_trust_env(self) -> None: + """Production session must not pick up env/.netrc credentials.""" + client = RestClient() + self.assertFalse(getattr(client._session, "trust_env", True)) + + def test_openapi_schema_rejects_wrong_parameter_types_before_http(self) -> None: + """OpenAPI-derived types are enforced; invalid args never hit REST.""" + client = self._rest() + tag_schema = input_schema_for_tool("get_documents_by_tag") + self.assertEqual(tag_schema["properties"]["tag"]["type"], "array") + with self.assertRaises(RestRequestError) as tag_ctx: + client.call_tool("get_documents_by_tag", {"tag": "not-an-array"}) + self.assertIn("Invalid arguments", str(tag_ctx.exception)) + + standards_schema = input_schema_for_tool("list_standards") + self.assertEqual(standards_schema["properties"]["all"]["type"], "boolean") + with self.assertRaises(RestRequestError) as all_ctx: + client.call_tool("list_standards", {"all": "true"}) + self.assertIn("Invalid arguments", str(all_ctx.exception)) + + # Confirm rejection happens before the HTTP adapter is invoked. + class _NoHttpSession: + cookies = None + + def get(self, *args: Any, **kwargs: Any) -> Any: + raise AssertionError("HTTP must not be called for invalid schema args") + + guarded = RestClient(session=_NoHttpSession()) + with self.assertRaises(RestRequestError): + guarded.call_tool("get_documents_by_tag", {"tag": "scalar"}) + with self.assertRaises(RestRequestError): + guarded.call_tool("list_standards", {"all": "yes"}) + + # --- E. Server list/call smoke (in-process) --- + + def test_server_lists_and_calls_tool(self) -> None: + self._seed_cre_and_node() + client = self._rest() + server = build_server(rest_client=client) + + async def _run() -> None: + list_handler = server.get_request_handler("tools/list") + self.assertIsNotNone(list_handler) + assert list_handler is not None + listed_raw = await list_handler.handler(None, None) + assert isinstance(listed_raw, types.ListToolsResult) + tool_names = [tool.name for tool in listed_raw.tools] + self.assertEqual(tool_names, APPROVED_TOOLS) + for tool in listed_raw.tools: + self.assertNotIn("format", (tool.input_schema.get("properties") or {})) + + call_handler = server.get_request_handler("tools/call") + self.assertIsNotNone(call_handler) + assert call_handler is not None + + result_raw = await call_handler.handler( + None, + types.CallToolRequestParams( + name="get_cre_by_id", arguments={"creid": "111-111"} + ), + ) + assert isinstance(result_raw, types.CallToolResult) + self.assertFalse(result_raw.is_error) + first = result_raw.content[0] + assert isinstance(first, types.TextContent) + payload = json.loads(first.text) + self.assertIn("data", payload) + + missing_raw = await call_handler.handler( + None, + types.CallToolRequestParams( + name="get_cre_by_id", arguments={"creid": "no-such"} + ), + ) + assert isinstance(missing_raw, types.CallToolResult) + self.assertTrue(missing_raw.is_error) + + unknown_raw = await call_handler.handler( + None, + types.CallToolRequestParams(name="map_analysis", arguments={}), + ) + assert isinstance(unknown_raw, types.CallToolResult) + self.assertTrue(unknown_raw.is_error) + + import asyncio + + asyncio.run(_run()) + + def test_unexpected_internal_errors_are_sanitized(self) -> None: + """Unexpected exceptions are logged server-side but not leaked to MCP clients.""" + sensitive = "SENSITIVE_INTERNAL_MARKER_do_not_leak" + + class _BoomSession: + cookies = None + + def get(self, *args: Any, **kwargs: Any) -> Any: + raise RuntimeError(sensitive) + + server = build_server(rest_client=RestClient(session=_BoomSession())) + + async def _run() -> None: + call_handler = server.get_request_handler("tools/call") + self.assertIsNotNone(call_handler) + assert call_handler is not None + result_raw = await call_handler.handler( + None, + types.CallToolRequestParams(name="list_ga_standards", arguments={}), + ) + assert isinstance(result_raw, types.CallToolResult) + self.assertTrue(result_raw.is_error) + first = result_raw.content[0] + assert isinstance(first, types.TextContent) + self.assertEqual(first.text, "Internal MCP tool error.") + self.assertNotIn(sensitive, first.text) + + import asyncio + + with self.assertLogs("application.mcp.server", level="ERROR"): + asyncio.run(_run()) + + def test_base_url_comes_from_env_not_tool_args(self) -> None: + with patch.dict(os.environ, {"OPENCRE_BASE_URL": "http://example.test:9"}): + client = RestClient() + self.assertEqual(client.base_url, "http://example.test:9") + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/api/mcp.md b/docs/api/mcp.md new file mode 100644 index 000000000..a10985f16 --- /dev/null +++ b/docs/api/mcp.md @@ -0,0 +1,106 @@ +# OpenCRE MCP server (issue #1003 v1) + +Local **stdio** MCP adapter that exposes a fixed allowlist of **public JSON REST reads**. + +## Scope (first PR) + +- Public `/rest/v1` GET tools that already work without login +- OpenAPI (`docs/api/openapi.yaml`) is the source of truth for each tool's **input** parameter schema +- No authentication, cookies, PAT, OAuth, or session forwarding +- No MCP bypass secret + +**Deferred to a later PR:** MyOpenCRE (`/rest/v1/user/resources`), chat (`/rest/v1/completion`), admin/import tools, and any credentialed flows. + +## Installation + +From the repo root, use the project virtualenv and install development dependencies (includes the MCP SDK): + +```bash +python3 -m venv venv +source venv/bin/activate +pip install -r requirements-dev.txt +``` + +## Run OpenCRE REST locally + +```bash +make docker-postgres # if using local Postgres +make migrate-upgrade +make dev-flask # http://127.0.0.1:5000 +``` + +Or point the MCP server at a hosted instance such as `https://opencre.org`. + +## Configuration + +| Variable | Meaning | Default | +|----------|---------|---------| +| `OPENCRE_BASE_URL` | OpenCRE origin used for REST calls (server-side only) | `http://127.0.0.1:5000` | + +Tool arguments cannot override the base URL. + +## Start the MCP server + +```bash +source venv/bin/activate +export OPENCRE_BASE_URL=http://127.0.0.1:5000 +python -m application.mcp +``` + +## Cursor setup (stdio) + +Add an MCP server entry that runs the module with your venv interpreter. Example (adjust the absolute repo path): + +```json +{ + "mcpServers": { + "opencre": { + "command": "/absolute/path/to/OpenCRE/venv/bin/python", + "args": ["-m", "application.mcp"], + "cwd": "/absolute/path/to/OpenCRE", + "env": { + "OPENCRE_BASE_URL": "http://127.0.0.1:5000" + } + } + } +} +``` + +## Tool ↔ REST mapping + +| MCP tool | HTTP method | REST path | +|----------|-------------|-----------| +| `get_cre_by_id` | GET | `/rest/v1/id/{creid}` | +| `get_cre_by_name` | GET | `/rest/v1/name/{crename}` | +| `get_node` | GET | `/rest/v1/{ntype}/{name}` | +| `get_documents_by_tag` | GET | `/rest/v1/tags` | +| `text_search` | GET | `/rest/v1/text_search` | +| `list_root_cres` | GET | `/rest/v1/root_cres` | +| `list_all_cres` | GET | `/rest/v1/all_cres` | +| `list_standards` | GET | `/rest/v1/standards` | +| `list_ga_standards` | GET | `/rest/v1/ga_standards` | + +Parameter names, types, requiredness, and descriptions for these tools are derived from the matching OpenAPI operations. The `format` query parameter is intentionally omitted so MCP tools stay JSON-oriented. + +## Deliberate first-PR parity gaps + +Not exposed yet (tracked for follow-up, not implied as MCP-complete): + +- Node path variants: `/section/`, `/sectionid/`, `/subsection/` +- Gap analysis: `map_analysis`, `map_analysis_weak_links`, `ma_job_results` +- `GET /rest/v1/cre_csv` (binary CSV) +- `GET /rest/v1/config` +- `GET /rest/v1/health` (feature-flagged ops probe) +- Deeplink redirect routes +- `GET /rest/v1/openapi.yaml` as a tool +- `/api/capabilities` +- Authenticated MyOpenCRE / chat / admin surfaces + +## Security boundary + +- Fixed public GET allowlist in `application/mcp/catalog.py` — OpenAPI membership alone does **not** expose a tool +- No credentials forwarded +- No MCP bypass secret +- Callers cannot supply arbitrary methods, paths, or base URLs +- `get_node` only accepts Credoctypes for `ntype` (blocks collisions such as `/rest/v1/user/resources`) +- REST remains the backend execution and authorization boundary diff --git a/requirements-dev.txt b/requirements-dev.txt index 4b8784bdc..7121de20e 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -86,3 +86,6 @@ mypy pytest pytest-base-url pytest-playwright + +# Local stdio MCP server (issue #1003 v1) — not needed on Heroku slug +mcp==2.0.0 From 110b0694c3e893f45643e9a0044da3759534668f Mon Sep 17 00:00:00 2001 From: nolanefe Date: Wed, 12 Aug 2026 20:21:57 +0300 Subject: [PATCH 2/2] fix: address CodeRabbit review feedback --- application/mcp/openapi_loader.py | 21 +++++-- application/mcp/rest_client.py | 15 ++++- application/mcp/server.py | 13 ++-- application/tests/mcp_public_tools_test.py | 73 +++++++++++++++++++++- 4 files changed, 105 insertions(+), 17 deletions(-) diff --git a/application/mcp/openapi_loader.py b/application/mcp/openapi_loader.py index 8fbd272f4..5fde907b8 100644 --- a/application/mcp/openapi_loader.py +++ b/application/mcp/openapi_loader.py @@ -34,15 +34,27 @@ def _load_spec(path: str) -> Dict[str, Any]: return data -def _resolve_ref(spec: Dict[str, Any], node: Any) -> Any: - """Resolve local #/components/... refs one level deep as needed.""" +def _resolve_ref( + spec: Dict[str, Any], + node: Any, + *, + _ref_stack: Optional[frozenset[str]] = None, +) -> Any: + """Resolve local #/components/... refs; reject cycles on the current ref chain.""" if not isinstance(node, dict): return node + stack = _ref_stack or frozenset() if "$ref" not in node: - return {key: _resolve_ref(spec, value) for key, value in node.items()} + # Sibling branches share the parent chain only — not each other's refs. + return { + key: _resolve_ref(spec, value, _ref_stack=stack) + for key, value in node.items() + } ref = node["$ref"] if not isinstance(ref, str) or not ref.startswith("#/"): raise OpenAPILookupError(f"Unsupported OpenAPI $ref: {ref}") + if ref in stack: + raise OpenAPILookupError(f"Circular OpenAPI $ref: {ref}") current: Any = spec for part in ref[2:].split("/"): if not isinstance(current, dict) or part not in current: @@ -50,12 +62,13 @@ def _resolve_ref(spec: Dict[str, Any], node: Any) -> Any: current = current[part] # Merge sibling keywords onto the resolved target (OpenAPI 3.0.3). resolved = copy.deepcopy(current) + next_stack = stack | {ref} if isinstance(resolved, dict): for key, value in node.items(): if key == "$ref": continue resolved[key] = value - return _resolve_ref(spec, resolved) + return _resolve_ref(spec, resolved, _ref_stack=next_stack) return resolved diff --git a/application/mcp/rest_client.py b/application/mcp/rest_client.py index 38550de0c..f5fb4e969 100644 --- a/application/mcp/rest_client.py +++ b/application/mcp/rest_client.py @@ -7,6 +7,7 @@ from __future__ import annotations import json +import logging import os import re from dataclasses import dataclass @@ -21,6 +22,8 @@ from application.mcp.catalog import ToolSpec, get_tool from application.mcp.openapi_loader import operation_input_schema +logger = logging.getLogger(__name__) + DEFAULT_BASE_URL = "http://127.0.0.1:5000" DEFAULT_TIMEOUT_SECONDS = 30.0 @@ -173,8 +176,13 @@ def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult: if hasattr(self._session, "cookies"): try: self._session.cookies.clear() - except Exception: - pass + except ( + Exception + ): # noqa: BLE001 - injected sessions may not be requests.Session + logger.warning( + "Failed to clear REST session cookies after tool call", + exc_info=True, + ) return _parse_response(response) @@ -299,7 +307,8 @@ def _parse_response(response: Any) -> RestResult: data: Any try: data = response.json() - except Exception: + except ValueError: + # JSON decode failures only — unrelated adapter errors must propagate. data = text if status < 200 or status >= 300: diff --git a/application/mcp/server.py b/application/mcp/server.py index 967186812..31429ff6e 100644 --- a/application/mcp/server.py +++ b/application/mcp/server.py @@ -10,11 +10,8 @@ from mcp.server.lowlevel import Server from mcp.server.stdio import stdio_server -from application.mcp.catalog import PUBLIC_TOOLS, get_tool, list_tool_names -from application.mcp.openapi_loader import ( - all_tool_input_schemas, - operation_input_schema, -) +from application.mcp.catalog import PUBLIC_TOOLS +from application.mcp.openapi_loader import all_tool_input_schemas from application.mcp.rest_client import ( RestClient, RestClientError, @@ -65,11 +62,9 @@ async def on_call_tool( name = params.name arguments: Dict[str, Any] = dict(params.arguments or {}) try: - if name not in list_tool_names(): + # schemas is built from the allowlist at startup; unknown names stop here. + if name not in schemas: raise RestRequestError(f"Unknown MCP tool: {name}") - # Ensure allowlist entry still matches OpenAPI before calling REST. - get_tool(name) - operation_input_schema(get_tool(name)) result = client.call_tool(name, arguments) payload = json.dumps(result.data, ensure_ascii=False, default=str) structured: Dict[str, Any] diff --git a/application/tests/mcp_public_tools_test.py b/application/tests/mcp_public_tools_test.py index 2ac4d3298..5355de7d7 100644 --- a/application/tests/mcp_public_tools_test.py +++ b/application/tests/mcp_public_tools_test.py @@ -17,6 +17,7 @@ from application.mcp.catalog import PUBLIC_TOOLS, get_tool, list_tool_names from application.mcp.openapi_loader import ( OpenAPILookupError, + _resolve_ref, all_tool_input_schemas, clear_openapi_cache, input_schema_for_tool, @@ -27,6 +28,7 @@ RestClient, RestRequestError, RestResponseError, + _parse_response, flask_test_session, ) from application.mcp.server import build_server, dispatch_tool @@ -52,7 +54,9 @@ def setUp(self) -> None: self.app = create_app(mode="test") self.app_context = self.app.app_context() self.app_context.push() - os.environ["INSECURE_REQUESTS"] = "True" + env_patcher = patch.dict(os.environ, {"INSECURE_REQUESTS": "True"}) + env_patcher.start() + self.addCleanup(env_patcher.stop) sqla.create_all() self.collection = db.Node_collection().with_graph() self.collection.graph.with_graph(graph=nx.DiGraph(), graph_data=[]) @@ -444,6 +448,73 @@ def test_base_url_comes_from_env_not_tool_args(self) -> None: client = RestClient() self.assertEqual(client.base_url, "http://example.test:9") + def test_parse_response_valueerror_falls_back_unrelated_propagates(self) -> None: + class _BadJson: + status_code = 200 + text = "not-json" + + def json(self) -> Any: + raise ValueError("No JSON object could be decoded") + + ok = _parse_response(_BadJson()) + self.assertEqual(ok.status_code, 200) + self.assertEqual(ok.data, "not-json") + + class _BoomJson: + status_code = 200 + text = "{}" + + def json(self) -> Any: + raise RuntimeError("adapter boom") + + with self.assertRaises(RuntimeError): + _parse_response(_BoomJson()) + + def test_circular_openapi_ref_raises_lookup_error(self) -> None: + cyclic = { + "components": { + "schemas": { + "A": {"$ref": "#/components/schemas/B"}, + "B": {"$ref": "#/components/schemas/A"}, + } + } + } + with self.assertRaises(OpenAPILookupError) as ctx: + _resolve_ref(cyclic, {"$ref": "#/components/schemas/A"}) + self.assertIn("Circular OpenAPI $ref", str(ctx.exception)) + + # Same $ref in sibling branches is not a cycle. + shared = { + "components": {"schemas": {"S": {"type": "string"}}}, + "properties": { + "left": {"$ref": "#/components/schemas/S"}, + "right": {"$ref": "#/components/schemas/S"}, + }, + } + resolved = _resolve_ref(shared, shared["properties"]) + self.assertEqual(resolved["left"]["type"], "string") + self.assertEqual(resolved["right"]["type"], "string") + + def test_unknown_mcp_tool_message_unchanged_after_server_simplify(self) -> None: + server = build_server(rest_client=self._rest()) + + async def _run() -> None: + call_handler = server.get_request_handler("tools/call") + assert call_handler is not None + result_raw = await call_handler.handler( + None, + types.CallToolRequestParams(name="map_analysis", arguments={}), + ) + assert isinstance(result_raw, types.CallToolResult) + self.assertTrue(result_raw.is_error) + first = result_raw.content[0] + assert isinstance(first, types.TextContent) + self.assertEqual(first.text, "Unknown MCP tool: map_analysis") + + import asyncio + + asyncio.run(_run()) + if __name__ == "__main__": unittest.main()