config validation, metadata certain keys should be promoted?
This commit is contained in:
@@ -33,6 +33,7 @@ from .models import (
|
|||||||
RawWorkflowPlan,
|
RawWorkflowPlan,
|
||||||
)
|
)
|
||||||
from .mcp_sdk_adapter import McpSdkAdapter
|
from .mcp_sdk_adapter import McpSdkAdapter
|
||||||
|
from .proxy_validation import ProxyConfigError, validate_transparent_proxy_config
|
||||||
from .service import WfMcpService
|
from .service import WfMcpService
|
||||||
from .store import FileStore, Store
|
from .store import FileStore, Store
|
||||||
from .transparent_proxy import (
|
from .transparent_proxy import (
|
||||||
@@ -61,6 +62,7 @@ __all__ = [
|
|||||||
"FileStore",
|
"FileStore",
|
||||||
"McpEvent",
|
"McpEvent",
|
||||||
"McpSdkAdapter",
|
"McpSdkAdapter",
|
||||||
|
"ProxyConfigError",
|
||||||
"RawWorkflowPlan",
|
"RawWorkflowPlan",
|
||||||
"Store",
|
"Store",
|
||||||
"ToolCallResult",
|
"ToolCallResult",
|
||||||
@@ -79,5 +81,6 @@ __all__ = [
|
|||||||
"run_broker_server",
|
"run_broker_server",
|
||||||
"run_transparent_proxy_server",
|
"run_transparent_proxy_server",
|
||||||
"specs_from_discovered_tools",
|
"specs_from_discovered_tools",
|
||||||
|
"validate_transparent_proxy_config",
|
||||||
"wrap_discovered_tool",
|
"wrap_discovered_tool",
|
||||||
]
|
]
|
||||||
|
|||||||
@@ -0,0 +1,131 @@
|
|||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from collections.abc import Iterable
|
||||||
|
|
||||||
|
from .models import BrokerConfig, ConnectionConfig
|
||||||
|
|
||||||
|
_NAMESPACE_ID_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9.-]*$")
|
||||||
|
_SUPPORTED_TRANSPORTS = {"stdio", "http", "streamable-http", "streamable_http", "sse"}
|
||||||
|
|
||||||
|
|
||||||
|
class ProxyConfigError(ValueError):
|
||||||
|
"""Raised when a broker config cannot safely run as a transparent proxy."""
|
||||||
|
|
||||||
|
|
||||||
|
def validate_transparent_proxy_config(
|
||||||
|
config: BrokerConfig,
|
||||||
|
*,
|
||||||
|
resources_as_tools: bool = False,
|
||||||
|
prompts_as_tools: bool = False,
|
||||||
|
) -> None:
|
||||||
|
errors: list[str] = []
|
||||||
|
_validate_connection_ids(config.connections, errors)
|
||||||
|
for connection in config.connections:
|
||||||
|
if connection.enabled:
|
||||||
|
_validate_connection_metadata(connection, errors)
|
||||||
|
_validate_reserved_tool_collisions(
|
||||||
|
config.connections,
|
||||||
|
errors,
|
||||||
|
resources_as_tools=resources_as_tools,
|
||||||
|
prompts_as_tools=prompts_as_tools,
|
||||||
|
)
|
||||||
|
if errors:
|
||||||
|
joined = "\n".join(f"- {error}" for error in errors)
|
||||||
|
raise ProxyConfigError(f"invalid transparent proxy config:\n{joined}")
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_connection_ids(
|
||||||
|
connections: Iterable[ConnectionConfig],
|
||||||
|
errors: list[str],
|
||||||
|
) -> None:
|
||||||
|
seen: dict[str, str] = {}
|
||||||
|
for connection in connections:
|
||||||
|
connection_id = connection.id
|
||||||
|
if connection_id in seen:
|
||||||
|
errors.append(f"duplicate connection id {connection_id!r}")
|
||||||
|
continue
|
||||||
|
seen[connection_id] = connection_id
|
||||||
|
|
||||||
|
if not connection_id:
|
||||||
|
errors.append("connection id must not be empty")
|
||||||
|
continue
|
||||||
|
if "_" in connection_id:
|
||||||
|
errors.append(
|
||||||
|
f"connection id {connection_id!r} must not contain '_' because "
|
||||||
|
"FastMCP Namespace uses '_' as the tool-name separator"
|
||||||
|
)
|
||||||
|
if not _NAMESPACE_ID_RE.fullmatch(connection_id):
|
||||||
|
errors.append(
|
||||||
|
f"connection id {connection_id!r} must contain only letters, "
|
||||||
|
"digits, dots, and hyphens, and must start with a letter or digit"
|
||||||
|
)
|
||||||
|
if "." not in connection_id:
|
||||||
|
errors.append(
|
||||||
|
f"connection id {connection_id!r} must look like '<server>.<account>'"
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_connection_metadata(
|
||||||
|
connection: ConnectionConfig,
|
||||||
|
errors: list[str],
|
||||||
|
) -> None:
|
||||||
|
metadata = connection.metadata
|
||||||
|
transport = metadata.get("transport", "stdio")
|
||||||
|
if not isinstance(transport, str):
|
||||||
|
errors.append(f"{connection.id}: metadata.transport must be a string")
|
||||||
|
return
|
||||||
|
if transport not in _SUPPORTED_TRANSPORTS:
|
||||||
|
errors.append(f"{connection.id}: unsupported MCP transport {transport!r}")
|
||||||
|
return
|
||||||
|
|
||||||
|
if transport == "stdio":
|
||||||
|
command = metadata.get("command")
|
||||||
|
if not isinstance(command, str) or not command:
|
||||||
|
errors.append(f"{connection.id}: stdio transport requires metadata.command")
|
||||||
|
args = metadata.get("args", [])
|
||||||
|
if not isinstance(args, list) or not all(isinstance(arg, str) for arg in args):
|
||||||
|
errors.append(f"{connection.id}: metadata.args must be a list of strings")
|
||||||
|
env = metadata.get("env", {})
|
||||||
|
if not isinstance(env, dict) or not all(
|
||||||
|
isinstance(key, str) and isinstance(value, str)
|
||||||
|
for key, value in env.items()
|
||||||
|
):
|
||||||
|
errors.append(f"{connection.id}: metadata.env must be a string map")
|
||||||
|
cwd = metadata.get("cwd")
|
||||||
|
if cwd is not None and not isinstance(cwd, str):
|
||||||
|
errors.append(f"{connection.id}: metadata.cwd must be a string when set")
|
||||||
|
return
|
||||||
|
|
||||||
|
url = metadata.get("url")
|
||||||
|
if not isinstance(url, str) or not url:
|
||||||
|
errors.append(f"{connection.id}: {transport} transport requires metadata.url")
|
||||||
|
headers = metadata.get("headers", {})
|
||||||
|
if not isinstance(headers, dict) or not all(
|
||||||
|
isinstance(key, str) and isinstance(value, str)
|
||||||
|
for key, value in headers.items()
|
||||||
|
):
|
||||||
|
errors.append(f"{connection.id}: metadata.headers must be a string map")
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_reserved_tool_collisions(
|
||||||
|
connections: Iterable[ConnectionConfig],
|
||||||
|
errors: list[str],
|
||||||
|
*,
|
||||||
|
resources_as_tools: bool,
|
||||||
|
prompts_as_tools: bool,
|
||||||
|
) -> None:
|
||||||
|
reserved_tools: set[str] = set()
|
||||||
|
if resources_as_tools:
|
||||||
|
reserved_tools.update({"list_resources", "read_resource"})
|
||||||
|
if prompts_as_tools:
|
||||||
|
reserved_tools.update({"list_prompts", "get_prompt"})
|
||||||
|
if not reserved_tools:
|
||||||
|
return
|
||||||
|
|
||||||
|
for connection in connections:
|
||||||
|
if connection.enabled and connection.id in {"list", "read", "get"}:
|
||||||
|
errors.append(
|
||||||
|
f"connection id {connection.id!r} is reserved when compatibility "
|
||||||
|
"resource/prompt tools are enabled"
|
||||||
|
)
|
||||||
@@ -11,6 +11,7 @@ from fastmcp.server import create_proxy
|
|||||||
from fastmcp.server.transforms import Namespace, PromptsAsTools, ResourcesAsTools
|
from fastmcp.server.transforms import Namespace, PromptsAsTools, ResourcesAsTools
|
||||||
|
|
||||||
from .models import BrokerConfig, ConnectionConfig
|
from .models import BrokerConfig, ConnectionConfig
|
||||||
|
from .proxy_validation import validate_transparent_proxy_config
|
||||||
|
|
||||||
|
|
||||||
def connection_to_fastmcp_server_config(
|
def connection_to_fastmcp_server_config(
|
||||||
@@ -40,6 +41,7 @@ def connection_to_fastmcp_server_config(
|
|||||||
|
|
||||||
|
|
||||||
def broker_config_to_fastmcp_config(config: BrokerConfig) -> MCPConfig:
|
def broker_config_to_fastmcp_config(config: BrokerConfig) -> MCPConfig:
|
||||||
|
validate_transparent_proxy_config(config)
|
||||||
return MCPConfig.from_dict(
|
return MCPConfig.from_dict(
|
||||||
{
|
{
|
||||||
"mcpServers": {
|
"mcpServers": {
|
||||||
@@ -57,6 +59,11 @@ def create_transparent_proxy_server(
|
|||||||
resources_as_tools: bool = False,
|
resources_as_tools: bool = False,
|
||||||
prompts_as_tools: bool = False,
|
prompts_as_tools: bool = False,
|
||||||
) -> FastMCP[Any]:
|
) -> FastMCP[Any]:
|
||||||
|
validate_transparent_proxy_config(
|
||||||
|
config,
|
||||||
|
resources_as_tools=resources_as_tools,
|
||||||
|
prompts_as_tools=prompts_as_tools,
|
||||||
|
)
|
||||||
root = FastMCP(
|
root = FastMCP(
|
||||||
"wf-mcp-transparent-proxy",
|
"wf-mcp-transparent-proxy",
|
||||||
instructions=(
|
instructions=(
|
||||||
|
|||||||
@@ -3,7 +3,15 @@ from __future__ import annotations
|
|||||||
import asyncio
|
import asyncio
|
||||||
import sys
|
import sys
|
||||||
|
|
||||||
from wf_mcp import BrokerConfig, ConnectionConfig, create_transparent_proxy_client
|
import pytest
|
||||||
|
|
||||||
|
from wf_mcp import (
|
||||||
|
BrokerConfig,
|
||||||
|
ConnectionConfig,
|
||||||
|
ProxyConfigError,
|
||||||
|
create_transparent_proxy_client,
|
||||||
|
validate_transparent_proxy_config,
|
||||||
|
)
|
||||||
|
|
||||||
from tests.test_wf_mcp_support import fixture_server_path, local_temp_root
|
from tests.test_wf_mcp_support import fixture_server_path, local_temp_root
|
||||||
|
|
||||||
@@ -41,6 +49,48 @@ def test_transparent_proxy_lists_and_calls_upstream_tools() -> None:
|
|||||||
asyncio.run(run_proxy())
|
asyncio.run(run_proxy())
|
||||||
|
|
||||||
|
|
||||||
|
def test_transparent_proxy_rejects_invalid_connection_config() -> None:
|
||||||
|
config = BrokerConfig(
|
||||||
|
store_root=local_temp_root() / "transparent_proxy_invalid_store",
|
||||||
|
connections=[
|
||||||
|
ConnectionConfig(
|
||||||
|
id="fixture.personal",
|
||||||
|
server="fixture",
|
||||||
|
account="personal",
|
||||||
|
metadata={"transport": "stdio"},
|
||||||
|
),
|
||||||
|
ConnectionConfig(
|
||||||
|
id="fixture.personal",
|
||||||
|
server="fixture",
|
||||||
|
account="work",
|
||||||
|
metadata={"transport": "websocket"},
|
||||||
|
),
|
||||||
|
ConnectionConfig(
|
||||||
|
id="bad_scope.personal",
|
||||||
|
server="bad_scope",
|
||||||
|
account="personal",
|
||||||
|
metadata={"transport": "stdio", "command": sys.executable},
|
||||||
|
),
|
||||||
|
ConnectionConfig(
|
||||||
|
id="fixture.http",
|
||||||
|
server="fixture",
|
||||||
|
account="http",
|
||||||
|
metadata={"transport": "http"},
|
||||||
|
),
|
||||||
|
],
|
||||||
|
)
|
||||||
|
|
||||||
|
with pytest.raises(ProxyConfigError) as exc_info:
|
||||||
|
validate_transparent_proxy_config(config)
|
||||||
|
|
||||||
|
message = str(exc_info.value)
|
||||||
|
assert "duplicate connection id 'fixture.personal'" in message
|
||||||
|
assert "fixture.personal: stdio transport requires metadata.command" in message
|
||||||
|
assert "fixture.personal: unsupported MCP transport 'websocket'" in message
|
||||||
|
assert "connection id 'bad_scope.personal' must not contain '_'" in message
|
||||||
|
assert "fixture.http: http transport requires metadata.url" in message
|
||||||
|
|
||||||
|
|
||||||
def test_transparent_proxy_can_expose_resources_and_prompts_as_tools() -> None:
|
def test_transparent_proxy_can_expose_resources_and_prompts_as_tools() -> None:
|
||||||
config = BrokerConfig(
|
config = BrokerConfig(
|
||||||
store_root=local_temp_root() / "transparent_proxy_helper_store",
|
store_root=local_temp_root() / "transparent_proxy_helper_store",
|
||||||
|
|||||||
Reference in New Issue
Block a user