fix(mcp): clear allowed_tools and tool overrides on MCP server edit (#29411)
* fix(mcp): clear allowed_tools and tool overrides on MCP server edit Send empty arrays/objects from the dashboard instead of null, coerce legacy null payloads before Prisma, and stop auto-selecting all tools when the stored allowlist is empty. Co-authored-by: Cursor <cursoragent@cursor.com> * style(mcp): simplify CRUD panel value ternary per review Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): enforce empty tool allowlist when cleared in dashboard Set mcp_info.tool_allowlist_enforced on UI save so [] blocks all tools while legacy servers with default [] remain unrestricted. Co-authored-by: Cursor <cursoragent@cursor.com> * Fix legacy MCP tool allowlist edit state * test(mcp): pin allowlist fields on mock server in tools test MagicMock auto-attributes are truthy and trigger server_applies_tool_allowlist after the empty-allowlist enforcement change. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): avoid locking legacy servers on quick edit save Only set tool_allowlist_enforced when already enforced or the user selected tools; skip allowlist fields on save for unrestricted servers; do not auto-select all tools when editing legacy servers before load. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): type mcp_info base for allowlist flag read Co-authored-by: Cursor <cursoragent@cursor.com> * fix(mcp): use MCPInfo type for tool_allowlist_enforced in edit save Co-authored-by: Cursor <cursoragent@cursor.com> * Update ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx Co-authored-by: veria-ai[bot] <224490171+veria-ai[bot]@users.noreply.github.com> * Remove unused MCP allowlist variable * Fix MCP legacy tool state display * Fix legacy MCP tool allowlist saves * fix(mcp): enforce allowlist when create flow deselects all tools Track explicit allowlist interaction in the create form so deselecting every tool persists tool_allowlist_enforced=true. Previously an empty selection sent the flag as false with allowed_tools=[], which the proxy treats as allow-all, contradicting the UI's 0 tools enabled state. This mirrors the existing edit-flow handling. * fix(mcp): enforce disallowed_tools on REST listing and keep restored tool selection on legacy edit --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: veria-ai[bot] <224490171+veria-ai[bot]@users.noreply.github.com> Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
This commit is contained in:
Sameer Kankute
GitHub
parent
e8fcb01215
commit
b7bbddbd4d
@ -67,6 +67,17 @@ def _prepare_mcp_server_data(
|
||||
# ``alias=None`` is a valid request to clear the stored alias.
|
||||
if data_dict.get("alias") is None and "alias" not in fields_set:
|
||||
data_dict.pop("alias", None)
|
||||
# Prisma ``allowed_tools`` is a required String[]; ``null`` is invalid.
|
||||
# The UI sends null to clear a whitelist — treat that as ``[]``.
|
||||
if "allowed_tools" in data_dict and data_dict["allowed_tools"] is None:
|
||||
data_dict["allowed_tools"] = []
|
||||
# Json map fields use ``@default("{}")``; explicit null means clear overrides.
|
||||
for json_map_field in (
|
||||
"tool_name_to_display_name",
|
||||
"tool_name_to_description",
|
||||
):
|
||||
if json_map_field in data_dict and data_dict[json_map_field] is None:
|
||||
data_dict[json_map_field] = {}
|
||||
else:
|
||||
data_dict = data.model_dump(exclude_none=True)
|
||||
# Ensure alias is always present in the dict (even if None)
|
||||
@ -93,13 +104,13 @@ def _prepare_mcp_server_data(
|
||||
if data_dict.get("env") is not None:
|
||||
data_dict["env"] = safe_dumps(data_dict["env"])
|
||||
|
||||
if data_dict.get("tool_name_to_display_name") is not None:
|
||||
if "tool_name_to_display_name" in data_dict:
|
||||
data_dict["tool_name_to_display_name"] = safe_dumps(
|
||||
data_dict["tool_name_to_display_name"]
|
||||
data_dict["tool_name_to_display_name"] or {}
|
||||
)
|
||||
if data_dict.get("tool_name_to_description") is not None:
|
||||
if "tool_name_to_description" in data_dict:
|
||||
data_dict["tool_name_to_description"] = safe_dumps(
|
||||
data_dict["tool_name_to_description"]
|
||||
data_dict["tool_name_to_description"] or {}
|
||||
)
|
||||
|
||||
# mcp_access_groups is already List[str], no serialization needed
|
||||
|
||||
@ -2429,7 +2429,13 @@ class MCPServerManager:
|
||||
"""
|
||||
Check if the tool is allowed or banned for the given server
|
||||
"""
|
||||
if server.allowed_tools:
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
server_applies_tool_allowlist,
|
||||
)
|
||||
|
||||
if server_applies_tool_allowlist(server):
|
||||
if not server.allowed_tools:
|
||||
return False
|
||||
return (
|
||||
tool_name in server.allowed_tools
|
||||
or f"{server.name}-{tool_name}" in server.allowed_tools
|
||||
|
||||
@ -365,10 +365,9 @@ if MCP_AVAILABLE:
|
||||
user_api_key_auth=user_api_key_auth,
|
||||
)
|
||||
|
||||
# Filter tools based on allowed_tools configuration
|
||||
# Only filter if allowed_tools is explicitly configured (not None and not empty)
|
||||
if server.allowed_tools is not None and len(server.allowed_tools) > 0:
|
||||
tools = filter_tools_by_allowed_tools(tools, server)
|
||||
# Always apply allowed_tools/disallowed_tools so the blacklist is
|
||||
# enforced even when no allowlist is set (matches the SSE/HTTP path).
|
||||
tools = filter_tools_by_allowed_tools(tools, server)
|
||||
|
||||
# Filter tools based on user_api_key_auth.object_permission.mcp_tool_permissions
|
||||
# This provides per-key/team/org control over which tools can be accessed
|
||||
|
||||
@ -945,10 +945,16 @@ if MCP_AVAILABLE:
|
||||
Returns:
|
||||
Filtered list of tools
|
||||
"""
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
server_applies_tool_allowlist,
|
||||
)
|
||||
|
||||
tools_to_return = tools
|
||||
|
||||
# Filter by allowed_tools (whitelist)
|
||||
if mcp_server.allowed_tools:
|
||||
if server_applies_tool_allowlist(mcp_server):
|
||||
if not mcp_server.allowed_tools:
|
||||
return []
|
||||
tools_to_return = [
|
||||
tool
|
||||
for tool in tools
|
||||
|
||||
@ -2,6 +2,7 @@
|
||||
MCP Server Utilities
|
||||
"""
|
||||
|
||||
import json
|
||||
import re
|
||||
from typing import Any, Dict, Iterator, Mapping, Optional, Tuple, Union
|
||||
|
||||
@ -162,6 +163,36 @@ def lookup_mcp_server_auth_in_headers(
|
||||
return None
|
||||
|
||||
|
||||
MCP_TOOL_ALLOWLIST_ENFORCED_KEY = "tool_allowlist_enforced"
|
||||
|
||||
|
||||
def _parse_mcp_info_dict(mcp_info: Any) -> Optional[Dict[str, Any]]:
|
||||
if mcp_info is None:
|
||||
return None
|
||||
if isinstance(mcp_info, dict):
|
||||
return mcp_info
|
||||
if isinstance(mcp_info, str):
|
||||
try:
|
||||
parsed = json.loads(mcp_info)
|
||||
except (ValueError, TypeError):
|
||||
return None
|
||||
return parsed if isinstance(parsed, dict) else None
|
||||
return None
|
||||
|
||||
|
||||
def is_server_tool_allowlist_enforced(mcp_server: Any) -> bool:
|
||||
mcp_info = _parse_mcp_info_dict(getattr(mcp_server, "mcp_info", None))
|
||||
if not mcp_info:
|
||||
return False
|
||||
return bool(mcp_info.get(MCP_TOOL_ALLOWLIST_ENFORCED_KEY))
|
||||
|
||||
|
||||
def server_applies_tool_allowlist(mcp_server: Any) -> bool:
|
||||
"""Whether server-level allowed_tools whitelist filtering is active."""
|
||||
allowed_tools = getattr(mcp_server, "allowed_tools", None) or []
|
||||
return is_server_tool_allowlist_enforced(mcp_server) or bool(allowed_tools)
|
||||
|
||||
|
||||
def validate_and_normalize_mcp_server_payload(payload: Any) -> None:
|
||||
"""
|
||||
Validate and normalize MCP server payload fields (server_name and alias).
|
||||
|
||||
@ -1862,9 +1862,11 @@ async def test_get_tools_for_single_server():
|
||||
)
|
||||
from mcp.types import Tool as MCPTool
|
||||
|
||||
# Create a mock server
|
||||
# Create a mock server (pin allowlist fields; MagicMock auto-attrs are truthy)
|
||||
mock_server = MagicMock()
|
||||
mock_server.mcp_info = {"server_name": "zapier"}
|
||||
mock_server.allowed_tools = None
|
||||
mock_server.disallowed_tools = None
|
||||
|
||||
# Create mock tools
|
||||
mock_tools = [
|
||||
@ -1899,6 +1901,44 @@ async def test_get_tools_for_single_server():
|
||||
assert result[0].mcp_info == {"server_name": "zapier"}
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_tools_for_single_server_applies_disallowed_tools_without_allowlist():
|
||||
"""REST listing must honor disallowed_tools even when no allowlist is set."""
|
||||
from litellm.proxy._experimental.mcp_server.rest_endpoints import (
|
||||
_get_tools_for_single_server,
|
||||
)
|
||||
from mcp.types import Tool as MCPTool
|
||||
|
||||
mock_server = MagicMock()
|
||||
mock_server.mcp_info = {"server_name": "zapier"}
|
||||
mock_server.name = "zapier"
|
||||
mock_server.server_id = "zapier"
|
||||
mock_server.allowed_tools = None
|
||||
mock_server.disallowed_tools = ["send_email"]
|
||||
|
||||
mock_tools = [
|
||||
MCPTool(
|
||||
name="send_email",
|
||||
description="Send an email",
|
||||
inputSchema={"type": "object"},
|
||||
),
|
||||
MCPTool(
|
||||
name="read_email",
|
||||
description="Read an email",
|
||||
inputSchema={"type": "object"},
|
||||
),
|
||||
]
|
||||
|
||||
with patch(
|
||||
"litellm.proxy._experimental.mcp_server.rest_endpoints.global_mcp_server_manager"
|
||||
) as mock_manager:
|
||||
mock_manager._get_tools_from_server = AsyncMock(return_value=mock_tools)
|
||||
|
||||
result = await _get_tools_for_single_server(mock_server, "Bearer test_token")
|
||||
|
||||
assert [tool.name for tool in result] == ["read_email"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_tool_rest_api_with_server_specific_auth():
|
||||
"""Test list_tool_rest_api with server-specific auth headers."""
|
||||
|
||||
@ -68,6 +68,34 @@ async def test_partial_update_omits_unset_defaultful_fields():
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_partial_update_null_tool_name_maps_clear_to_empty_json():
|
||||
"""Explicit null on Json map fields must clear overrides (UI legacy)."""
|
||||
data = UpdateMCPServerRequest(
|
||||
server_id="my-test-server",
|
||||
tool_name_to_display_name=None,
|
||||
tool_name_to_description=None,
|
||||
)
|
||||
|
||||
data_dict = await _run_update(data)
|
||||
|
||||
assert data_dict["tool_name_to_display_name"] == "{}"
|
||||
assert data_dict["tool_name_to_description"] == "{}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_partial_update_null_allowed_tools_clears_whitelist():
|
||||
"""Explicit null must clear the whitelist (UI legacy); Prisma requires []."""
|
||||
data = UpdateMCPServerRequest(
|
||||
server_id="my-test-server",
|
||||
allowed_tools=None,
|
||||
)
|
||||
|
||||
data_dict = await _run_update(data)
|
||||
|
||||
assert data_dict["allowed_tools"] == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_partial_update_preserves_http_transport():
|
||||
"""The reported prod incident: a PUT without transport must not flip http->sse."""
|
||||
|
||||
@ -4184,6 +4184,85 @@ def test_filter_tools_by_allowed_tools_no_filter():
|
||||
assert len(filtered_tools) == 2
|
||||
|
||||
|
||||
def test_filter_tools_enforced_empty_allowlist_blocks_all():
|
||||
from mcp.types import Tool
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.server import (
|
||||
filter_tools_by_allowed_tools,
|
||||
)
|
||||
from litellm.types.mcp import MCPTransport
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
tools = [
|
||||
Tool(
|
||||
name="read_wiki_structure",
|
||||
title=None,
|
||||
description="",
|
||||
inputSchema={"type": "object"},
|
||||
outputSchema=None,
|
||||
annotations=None,
|
||||
),
|
||||
]
|
||||
server = MCPServer(
|
||||
server_id="deepwiki",
|
||||
name="deepwiki",
|
||||
transport=MCPTransport.http,
|
||||
allowed_tools=[],
|
||||
mcp_info={"tool_allowlist_enforced": True},
|
||||
)
|
||||
|
||||
assert filter_tools_by_allowed_tools(tools, server) == []
|
||||
|
||||
|
||||
def test_filter_tools_legacy_empty_allowlist_allows_all():
|
||||
from mcp.types import Tool
|
||||
|
||||
from litellm.proxy._experimental.mcp_server.server import (
|
||||
filter_tools_by_allowed_tools,
|
||||
)
|
||||
from litellm.types.mcp import MCPTransport
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
tools = [
|
||||
Tool(
|
||||
name="read_wiki_structure",
|
||||
title=None,
|
||||
description="",
|
||||
inputSchema={"type": "object"},
|
||||
outputSchema=None,
|
||||
annotations=None,
|
||||
),
|
||||
]
|
||||
server = MCPServer(
|
||||
server_id="legacy",
|
||||
name="legacy",
|
||||
transport=MCPTransport.http,
|
||||
allowed_tools=[],
|
||||
mcp_info=None,
|
||||
)
|
||||
|
||||
assert len(filter_tools_by_allowed_tools(tools, server)) == 1
|
||||
|
||||
|
||||
def test_check_allowed_or_banned_tools_enforced_empty_denies_calls():
|
||||
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
|
||||
MCPServerManager,
|
||||
)
|
||||
from litellm.types.mcp import MCPTransport
|
||||
from litellm.types.mcp_server.mcp_server_manager import MCPServer
|
||||
|
||||
manager = MCPServerManager.__new__(MCPServerManager)
|
||||
server = MCPServer(
|
||||
server_id="deepwiki",
|
||||
name="deepwiki",
|
||||
transport=MCPTransport.http,
|
||||
allowed_tools=[],
|
||||
mcp_info={"tool_allowlist_enforced": True},
|
||||
)
|
||||
|
||||
assert manager.check_allowed_or_banned_tools("read_wiki_structure", server) is False
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_tools_from_mcp_servers_injects_stored_oauth2_token():
|
||||
"""
|
||||
@ -4540,9 +4619,9 @@ class TestEnsureUpstreamInitializeInstructionsCached:
|
||||
await global_mcp_server_manager._ensure_upstream_initialize_instructions_cached(
|
||||
server
|
||||
)
|
||||
assert create.await_count == 1, (
|
||||
"Second probe within cooldown must not reconnect to upstream"
|
||||
)
|
||||
assert (
|
||||
create.await_count == 1
|
||||
), "Second probe within cooldown must not reconnect to upstream"
|
||||
assert (
|
||||
"empty-server"
|
||||
not in global_mcp_server_manager._upstream_initialize_instructions_by_server_id
|
||||
@ -4567,7 +4646,9 @@ class TestEnsureUpstreamInitializeInstructionsCached:
|
||||
|
||||
server = _make_instruction_server(server_id="boom-server", instructions=None)
|
||||
fake_client = MagicMock()
|
||||
fake_client.run_with_session = AsyncMock(side_effect=RuntimeError("upstream down"))
|
||||
fake_client.run_with_session = AsyncMock(
|
||||
side_effect=RuntimeError("upstream down")
|
||||
)
|
||||
fake_client._last_initialize_instructions = None
|
||||
|
||||
create = AsyncMock(return_value=fake_client)
|
||||
@ -4579,9 +4660,9 @@ class TestEnsureUpstreamInitializeInstructionsCached:
|
||||
await global_mcp_server_manager._ensure_upstream_initialize_instructions_cached(
|
||||
server
|
||||
)
|
||||
assert create.await_count == 1, (
|
||||
"Second probe within cooldown must not reconnect after failure"
|
||||
)
|
||||
assert (
|
||||
create.await_count == 1
|
||||
), "Second probe within cooldown must not reconnect after failure"
|
||||
assert (
|
||||
"boom-server"
|
||||
not in global_mcp_server_manager._upstream_initialize_instructions_by_server_id
|
||||
|
||||
@ -27,7 +27,19 @@ vi.mock("./MCPPermissionManagement", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./mcp_tool_configuration", () => ({
|
||||
default: () => <div data-testid="mcp-tool-config" />,
|
||||
default: ({ onAllowedToolsChange, onToolAllowlistInteraction }: any) => (
|
||||
<div data-testid="mcp-tool-config">
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
onToolAllowlistInteraction?.();
|
||||
onAllowedToolsChange?.([]);
|
||||
}}
|
||||
>
|
||||
Disable all tools
|
||||
</button>
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
||||
vi.mock("./mcp_connection_status", () => ({
|
||||
@ -335,6 +347,50 @@ describe("CreateMCPServer", () => {
|
||||
// No credentials should be sent for "none" auth
|
||||
expect(payload.credentials).toBeUndefined();
|
||||
});
|
||||
|
||||
it("enforces the allowlist when the user explicitly deselects every tool", async () => {
|
||||
await selectHttpTransport();
|
||||
|
||||
const user = userEvent.setup({ delay: null });
|
||||
|
||||
const nameInput = getServerNameInput();
|
||||
await user.type(nameInput, "Locked_Down_Server");
|
||||
|
||||
const urlInput = screen.getByPlaceholderText("https://your-mcp-server.com");
|
||||
await user.type(urlInput, "https://example.com/mcp");
|
||||
|
||||
await selectAntOption("Authentication", "None");
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole("button", { name: "Disable all tools" }));
|
||||
});
|
||||
|
||||
vi.mocked(networking.createMCPServer).mockResolvedValue({
|
||||
server_id: "new-server-1",
|
||||
server_name: "Locked_Down_Server",
|
||||
alias: "Locked_Down_Server",
|
||||
url: "https://example.com/mcp",
|
||||
transport: "http",
|
||||
auth_type: "none",
|
||||
created_at: "2024-01-01T00:00:00Z",
|
||||
created_by: "user-1",
|
||||
updated_at: "2024-01-01T00:00:00Z",
|
||||
updated_by: "user-1",
|
||||
});
|
||||
|
||||
const submitButton = screen.getByRole("button", { name: "Add MCP Server" });
|
||||
await act(async () => {
|
||||
fireEvent.click(submitButton);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(networking.createMCPServer).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [, payload] = vi.mocked(networking.createMCPServer).mock.calls[0];
|
||||
expect(payload.mcp_info.tool_allowlist_enforced).toBe(true);
|
||||
expect(payload.allowed_tools).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("when OAuth interactive auth is selected", () => {
|
||||
|
||||
@ -69,6 +69,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
} | null>(null);
|
||||
const [aliasManuallyEdited, setAliasManuallyEdited] = useState(false);
|
||||
const [allowedTools, setAllowedTools] = useState<string[]>([]);
|
||||
const [hasToolAllowlistInteraction, setHasToolAllowlistInteraction] = useState(false);
|
||||
const [toolNameToDisplayName, setToolNameToDisplayName] = useState<Record<string, string>>({});
|
||||
const [toolNameToDescription, setToolNameToDescription] = useState<Record<string, string>>({});
|
||||
const [transportType, setTransportType] = useState<string>("");
|
||||
@ -106,6 +107,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
transportType,
|
||||
costConfig,
|
||||
allowedTools,
|
||||
hasToolAllowlistInteraction,
|
||||
searchValue,
|
||||
aliasManuallyEdited,
|
||||
logoUrl,
|
||||
@ -204,6 +206,9 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
if (parsed.allowedTools) {
|
||||
setAllowedTools(parsed.allowedTools);
|
||||
}
|
||||
if (typeof parsed.hasToolAllowlistInteraction === "boolean") {
|
||||
setHasToolAllowlistInteraction(parsed.hasToolAllowlistInteraction);
|
||||
}
|
||||
if (parsed.searchValue) {
|
||||
setSearchValue(parsed.searchValue);
|
||||
}
|
||||
@ -384,12 +389,13 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
description: restValues.description,
|
||||
logo_url: logoUrl || undefined,
|
||||
mcp_server_cost_info: Object.keys(costConfig).length > 0 ? costConfig : null,
|
||||
tool_allowlist_enforced: hasToolAllowlistInteraction || allowedTools.length > 0,
|
||||
},
|
||||
mcp_access_groups: accessGroups,
|
||||
alias: restValues.alias,
|
||||
allowed_tools: allowedTools.length > 0 ? allowedTools : null,
|
||||
tool_name_to_display_name: Object.keys(toolNameToDisplayName).length > 0 ? toolNameToDisplayName : null,
|
||||
tool_name_to_description: Object.keys(toolNameToDescription).length > 0 ? toolNameToDescription : null,
|
||||
allowed_tools: allowedTools,
|
||||
tool_name_to_display_name: toolNameToDisplayName,
|
||||
tool_name_to_description: toolNameToDescription,
|
||||
allow_all_keys: Boolean(allowAllKeysRaw),
|
||||
available_on_public_internet: Boolean(availableOnPublicInternetRaw),
|
||||
delegate_auth_to_upstream: Boolean(delegateAuthToUpstreamRaw),
|
||||
@ -436,6 +442,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
setCostConfig({});
|
||||
clearTools();
|
||||
setAllowedTools([]);
|
||||
setHasToolAllowlistInteraction(false);
|
||||
setAliasManuallyEdited(false);
|
||||
setLogoUrl(undefined);
|
||||
setModalVisible(false);
|
||||
@ -457,6 +464,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
setCostConfig({});
|
||||
clearTools();
|
||||
setAllowedTools([]);
|
||||
setHasToolAllowlistInteraction(false);
|
||||
setAliasManuallyEdited(false);
|
||||
setLogoUrl(undefined);
|
||||
setModalVisible(false);
|
||||
@ -1040,6 +1048,8 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
||||
allowedTools={allowedTools}
|
||||
existingAllowedTools={null}
|
||||
onAllowedToolsChange={setAllowedTools}
|
||||
hasToolAllowlistInteraction={hasToolAllowlistInteraction}
|
||||
onToolAllowlistInteraction={() => setHasToolAllowlistInteraction(true)}
|
||||
toolNameToDisplayName={toolNameToDisplayName}
|
||||
toolNameToDescription={toolNameToDescription}
|
||||
onToolNameToDisplayNameChange={setToolNameToDisplayName}
|
||||
|
||||
@ -35,7 +35,34 @@ vi.mock("./MCPPermissionManagement", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("./mcp_tool_configuration", () => ({
|
||||
default: () => <div data-testid="mcp-tool-config" />,
|
||||
default: ({
|
||||
existingAllowedTools,
|
||||
onAllowedToolsChange,
|
||||
onToolAllowlistInteraction,
|
||||
onToolNameToDisplayNameChange,
|
||||
onToolNameToDescriptionChange,
|
||||
}: any) => (
|
||||
<div data-testid="mcp-tool-config" data-existing-allowed-tools={JSON.stringify(existingAllowedTools)}>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
onToolAllowlistInteraction?.();
|
||||
onAllowedToolsChange([]);
|
||||
}}
|
||||
>
|
||||
Disable all tools
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => {
|
||||
onToolNameToDisplayNameChange({ read_user: "Read User" });
|
||||
onToolNameToDescriptionChange({ read_user: "Reads users" });
|
||||
}}
|
||||
>
|
||||
Set tool overrides
|
||||
</button>
|
||||
</div>
|
||||
),
|
||||
}));
|
||||
|
||||
// ── fixtures ──────────────────────────────────────────────────────────────────
|
||||
@ -43,7 +70,7 @@ vi.mock("./mcp_tool_configuration", () => ({
|
||||
const interactiveOAuthServer = {
|
||||
server_id: "oauth_server_1",
|
||||
server_name: "OAuthServer",
|
||||
alias: "oauth_server", // underscores: hyphens fail validateMCPServerName
|
||||
alias: "oauth_server", // underscores: hyphens fail validateMCPServerName
|
||||
description: "Interactive OAuth MCP server",
|
||||
transport: "http",
|
||||
url: "https://example.com/mcp",
|
||||
@ -218,6 +245,128 @@ describe("MCPServerEdit (delegate auth)", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCPServerEdit (tool allowlist)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
});
|
||||
|
||||
it("treats legacy empty allowed_tools as unrestricted", () => {
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
...interactiveOAuthServer,
|
||||
allowed_tools: [],
|
||||
mcp_info: { server_name: "OAuthServer" },
|
||||
}}
|
||||
accessToken={null}
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("mcp-tool-config")).toHaveAttribute("data-existing-allowed-tools", "null");
|
||||
});
|
||||
|
||||
it("honors enforced empty allowed_tools", () => {
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
...interactiveOAuthServer,
|
||||
allowed_tools: [],
|
||||
mcp_info: { server_name: "OAuthServer", tool_allowlist_enforced: true },
|
||||
}}
|
||||
accessToken={null}
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId("mcp-tool-config")).toHaveAttribute("data-existing-allowed-tools", "[]");
|
||||
});
|
||||
|
||||
it("saves an explicit empty allowlist after legacy unrestricted tools are disabled", async () => {
|
||||
vi.mocked(networking.updateMCPServer).mockResolvedValue({
|
||||
...interactiveOAuthServer,
|
||||
allowed_tools: [],
|
||||
mcp_info: { server_name: "OAuthServer", tool_allowlist_enforced: true },
|
||||
});
|
||||
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
...interactiveOAuthServer,
|
||||
allowed_tools: [],
|
||||
mcp_info: { server_name: "OAuthServer" },
|
||||
}}
|
||||
accessToken="access-token"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole("button", { name: "Disable all tools" }));
|
||||
});
|
||||
|
||||
const saveButtons = screen.getAllByRole("button", { name: "Save Changes" });
|
||||
await act(async () => {
|
||||
fireEvent.click(saveButtons[0]);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(networking.updateMCPServer).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0];
|
||||
expect(payload.mcp_info.tool_allowlist_enforced).toBe(true);
|
||||
expect(payload.allowed_tools).toEqual([]);
|
||||
});
|
||||
|
||||
it("saves tool overrides for legacy unrestricted servers", async () => {
|
||||
vi.mocked(networking.updateMCPServer).mockResolvedValue({
|
||||
...interactiveOAuthServer,
|
||||
tool_name_to_display_name: { read_user: "Read User" },
|
||||
tool_name_to_description: { read_user: "Reads users" },
|
||||
});
|
||||
|
||||
render(
|
||||
<MCPServerEdit
|
||||
mcpServer={{
|
||||
...interactiveOAuthServer,
|
||||
allowed_tools: [],
|
||||
mcp_info: { server_name: "OAuthServer" },
|
||||
}}
|
||||
accessToken="access-token"
|
||||
onCancel={vi.fn()}
|
||||
onSuccess={vi.fn()}
|
||||
availableAccessGroups={[]}
|
||||
/>,
|
||||
);
|
||||
|
||||
await act(async () => {
|
||||
fireEvent.click(screen.getByRole("button", { name: "Set tool overrides" }));
|
||||
});
|
||||
|
||||
const saveButtons = screen.getAllByRole("button", { name: "Save Changes" });
|
||||
await act(async () => {
|
||||
fireEvent.click(saveButtons[0]);
|
||||
});
|
||||
|
||||
await waitFor(() => {
|
||||
expect(networking.updateMCPServer).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
const [, payload] = vi.mocked(networking.updateMCPServer).mock.calls[0];
|
||||
expect(payload.mcp_info.tool_allowlist_enforced).toBe(false);
|
||||
expect(payload.allowed_tools).toBeUndefined();
|
||||
expect(payload.tool_name_to_display_name).toEqual({ read_user: "Read User" });
|
||||
expect(payload.tool_name_to_description).toEqual({ read_user: "Reads users" });
|
||||
});
|
||||
});
|
||||
|
||||
describe("MCPServerEdit (interactive OAuth)", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
|
||||
@ -41,6 +41,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
const [searchValue, setSearchValue] = useState<string>("");
|
||||
const [aliasManuallyEdited, setAliasManuallyEdited] = useState(false);
|
||||
const [allowedTools, setAllowedTools] = useState<string[]>([]);
|
||||
const [hasToolAllowlistInteraction, setHasToolAllowlistInteraction] = useState(false);
|
||||
const [toolNameToDisplayName, setToolNameToDisplayName] = useState<Record<string, string>>({});
|
||||
const [toolNameToDescription, setToolNameToDescription] = useState<Record<string, string>>({});
|
||||
const [pendingRestoredValues, setPendingRestoredValues] = useState<Record<string, any> | null>(null);
|
||||
@ -68,6 +69,9 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
const currentAuthorizationUrl = Form.useWatch("authorization_url", form);
|
||||
const currentTokenUrl = Form.useWatch("token_url", form);
|
||||
const currentRegistrationUrl = Form.useWatch("registration_url", form);
|
||||
const hasExistingToolAllowlist =
|
||||
Boolean(mcpServer.mcp_info?.tool_allowlist_enforced) || (mcpServer.allowed_tools?.length ?? 0) > 0;
|
||||
const existingAllowedTools = hasExistingToolAllowlist ? mcpServer.allowed_tools ?? [] : null;
|
||||
|
||||
const persistEditUiState = () => {
|
||||
if (typeof window === "undefined") {
|
||||
@ -82,6 +86,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
formValues: values,
|
||||
costConfig,
|
||||
allowedTools,
|
||||
hasToolAllowlistInteraction,
|
||||
searchValue,
|
||||
aliasManuallyEdited,
|
||||
}),
|
||||
@ -135,7 +140,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
},
|
||||
onTokenReceived: (token) => {
|
||||
setOauthAccessToken(token?.access_token ?? null);
|
||||
|
||||
|
||||
if (token?.access_token) {
|
||||
const credentials = {
|
||||
access_token: token.access_token,
|
||||
@ -143,11 +148,11 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
...(token.expires_in && { expires_in: token.expires_in }),
|
||||
...(token.scope && { scope: token.scope }),
|
||||
};
|
||||
|
||||
|
||||
form.setFieldsValue({ credentials });
|
||||
|
||||
|
||||
NotificationsManager.success(
|
||||
"OAuth authorization successful! Please click 'Update MCP Server' to save the credentials."
|
||||
"OAuth authorization successful! Please click 'Update MCP Server' to save the credentials.",
|
||||
);
|
||||
}
|
||||
},
|
||||
@ -176,7 +181,6 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
}
|
||||
}, [mcpServer.env]);
|
||||
|
||||
|
||||
// If server has spec_path, show it as "openapi" transport in the UI
|
||||
const effectiveTransport = React.useMemo(() => {
|
||||
if (mcpServer.spec_path && mcpServer.transport !== "stdio") {
|
||||
@ -208,12 +212,16 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
|
||||
// Initialize allowed tools and tool overrides from existing server data
|
||||
useEffect(() => {
|
||||
if (mcpServer.allowed_tools) {
|
||||
setAllowedTools(mcpServer.allowed_tools);
|
||||
setHasToolAllowlistInteraction(false);
|
||||
}, [mcpServer.server_id]);
|
||||
|
||||
useEffect(() => {
|
||||
if (hasExistingToolAllowlist) {
|
||||
setAllowedTools(mcpServer.allowed_tools ?? []);
|
||||
}
|
||||
setToolNameToDisplayName(mcpServer.tool_name_to_display_name ?? {});
|
||||
setToolNameToDescription(mcpServer.tool_name_to_description ?? {});
|
||||
}, [mcpServer]);
|
||||
}, [mcpServer, hasExistingToolAllowlist]);
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof window === "undefined") {
|
||||
@ -238,6 +246,9 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
if (parsed.allowedTools) {
|
||||
setAllowedTools(parsed.allowedTools);
|
||||
}
|
||||
if (typeof parsed.hasToolAllowlistInteraction === "boolean") {
|
||||
setHasToolAllowlistInteraction(parsed.hasToolAllowlistInteraction);
|
||||
}
|
||||
if (parsed.searchValue) {
|
||||
setSearchValue(parsed.searchValue);
|
||||
}
|
||||
@ -529,6 +540,8 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
mcpServer.alias ||
|
||||
"unknown";
|
||||
|
||||
const toolAllowlistEnforced = hasExistingToolAllowlist || hasToolAllowlistInteraction || allowedTools.length > 0;
|
||||
|
||||
const payload: Record<string, any> = {
|
||||
...restValues,
|
||||
...stdioFields,
|
||||
@ -537,16 +550,22 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
env_json: undefined,
|
||||
server_id: mcpServer.server_id,
|
||||
mcp_info: {
|
||||
...(mcpServer.mcp_info ?? {}),
|
||||
server_name: mcpInfoServerName,
|
||||
description: restValues.description,
|
||||
logo_url: logoUrl || undefined,
|
||||
mcp_server_cost_info: Object.keys(costConfig).length > 0 ? costConfig : null,
|
||||
tool_allowlist_enforced: toolAllowlistEnforced,
|
||||
},
|
||||
mcp_access_groups: accessGroups,
|
||||
alias: restValues.alias,
|
||||
// Include permission management fields
|
||||
extra_headers: restValues.extra_headers || [],
|
||||
allowed_tools: allowedTools.length > 0 ? allowedTools : null,
|
||||
...(toolAllowlistEnforced
|
||||
? {
|
||||
allowed_tools: allowedTools,
|
||||
}
|
||||
: {}),
|
||||
tool_name_to_display_name: Object.keys(toolNameToDisplayName).length > 0 ? toolNameToDisplayName : null,
|
||||
tool_name_to_description: Object.keys(toolNameToDescription).length > 0 ? toolNameToDescription : null,
|
||||
disallowed_tools: restValues.disallowed_tools || [],
|
||||
@ -563,12 +582,11 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
? Boolean(delegateAuthToUpstreamRaw ?? mcpServer.delegate_auth_to_upstream)
|
||||
: false,
|
||||
// Include token_validation when it is set (non-null) or when clearing an existing value
|
||||
...(tokenValidation !== null || mcpServer.token_validation
|
||||
? { token_validation: tokenValidation }
|
||||
: {}),
|
||||
...(tokenValidation !== null || mcpServer.token_validation ? { token_validation: tokenValidation } : {}),
|
||||
};
|
||||
|
||||
const includeCredentials = restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);
|
||||
const includeCredentials =
|
||||
restValues.auth_type && AUTH_TYPES_REQUIRING_CREDENTIALS.includes(restValues.auth_type);
|
||||
|
||||
if (includeCredentials && credentialsPayload && Object.keys(credentialsPayload).length > 0) {
|
||||
payload.credentials = credentialsPayload;
|
||||
@ -700,10 +718,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
/>
|
||||
</Form.Item>
|
||||
|
||||
<Form.Item
|
||||
label="Args"
|
||||
name="args"
|
||||
>
|
||||
<Form.Item label="Args" name="args">
|
||||
<Select
|
||||
mode="tags"
|
||||
size="large"
|
||||
@ -916,17 +931,15 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
}
|
||||
name="token_storage_ttl_seconds"
|
||||
>
|
||||
<InputNumber
|
||||
min={1}
|
||||
placeholder="e.g. 3600"
|
||||
style={{ width: "100%" }}
|
||||
className="rounded-lg"
|
||||
/>
|
||||
<InputNumber min={1} placeholder="e.g. 3600" style={{ width: "100%" }} className="rounded-lg" />
|
||||
</Form.Item>
|
||||
</>
|
||||
)}
|
||||
<div className="rounded-lg border border-dashed border-gray-300 p-4 space-y-2">
|
||||
<p className="text-sm text-gray-600">Use OAuth to fetch a fresh access token and temporarily save it in the session as the authentication value.</p>
|
||||
<p className="text-sm text-gray-600">
|
||||
Use OAuth to fetch a fresh access token and temporarily save it in the session as the authentication
|
||||
value.
|
||||
</p>
|
||||
<Button
|
||||
variant="secondary"
|
||||
onClick={startOAuthFlow}
|
||||
@ -952,7 +965,12 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
<>
|
||||
<p className="text-sm text-gray-500 mb-2">
|
||||
For MCP servers hosted on AWS Bedrock AgentCore.{" "}
|
||||
<a href="https://docs.litellm.ai/docs/mcp_aws_sigv4" target="_blank" rel="noopener noreferrer" className="text-blue-500 hover:text-blue-700">
|
||||
<a
|
||||
href="https://docs.litellm.ai/docs/mcp_aws_sigv4"
|
||||
target="_blank"
|
||||
rel="noopener noreferrer"
|
||||
className="text-blue-500 hover:text-blue-700"
|
||||
>
|
||||
View docs →
|
||||
</a>
|
||||
</p>
|
||||
@ -1098,7 +1116,7 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
transport: transportType ?? mcpServer.transport,
|
||||
auth_type: currentAuthType ?? mcpServer.auth_type,
|
||||
mcp_info: mcpServer.mcp_info,
|
||||
oauth_flow_type: (currentTokenUrl ?? mcpServer.token_url) ? OAUTH_FLOW.M2M : OAUTH_FLOW.INTERACTIVE,
|
||||
oauth_flow_type: currentTokenUrl ?? mcpServer.token_url ? OAUTH_FLOW.M2M : OAUTH_FLOW.INTERACTIVE,
|
||||
static_headers: currentStaticHeaders ?? mcpServer.static_headers,
|
||||
credentials: currentCredentials,
|
||||
authorization_url: currentAuthorizationUrl ?? mcpServer.authorization_url,
|
||||
@ -1106,8 +1124,11 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
|
||||
registration_url: currentRegistrationUrl ?? mcpServer.registration_url,
|
||||
}}
|
||||
allowedTools={allowedTools}
|
||||
existingAllowedTools={mcpServer.allowed_tools || null}
|
||||
existingAllowedTools={existingAllowedTools}
|
||||
hasToolAllowlistInteraction={hasToolAllowlistInteraction}
|
||||
isEditMode
|
||||
onAllowedToolsChange={setAllowedTools}
|
||||
onToolAllowlistInteraction={() => setHasToolAllowlistInteraction(true)}
|
||||
toolNameToDisplayName={toolNameToDisplayName}
|
||||
toolNameToDescription={toolNameToDescription}
|
||||
onToolNameToDisplayNameChange={setToolNameToDisplayName}
|
||||
|
||||
@ -0,0 +1,112 @@
|
||||
import { useState } from "react";
|
||||
import { fireEvent, render, screen, waitFor } from "@testing-library/react";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import MCPToolConfiguration from "./mcp_tool_configuration";
|
||||
|
||||
const tools = [
|
||||
{ name: "read_user", description: "Read user" },
|
||||
{ name: "delete_user", description: "Delete user" },
|
||||
];
|
||||
|
||||
const renderToolConfiguration = (onAllowedToolsChange = vi.fn()) => {
|
||||
render(
|
||||
<MCPToolConfiguration
|
||||
accessToken="token"
|
||||
formValues={{ url: "https://example.com/mcp", transport: "http", auth_type: "none" }}
|
||||
allowedTools={[]}
|
||||
existingAllowedTools={null}
|
||||
onAllowedToolsChange={onAllowedToolsChange}
|
||||
toolNameToDisplayName={{}}
|
||||
toolNameToDescription={{}}
|
||||
onToolNameToDisplayNameChange={vi.fn()}
|
||||
onToolNameToDescriptionChange={vi.fn()}
|
||||
externalTools={tools}
|
||||
externalCanFetch
|
||||
isEditMode
|
||||
/>,
|
||||
);
|
||||
|
||||
return onAllowedToolsChange;
|
||||
};
|
||||
|
||||
describe("MCPToolConfiguration", () => {
|
||||
it("shows legacy unrestricted edit tools enabled in flat view", async () => {
|
||||
const onAllowedToolsChange = renderToolConfiguration();
|
||||
|
||||
fireEvent.click(screen.getByText("Flat List"));
|
||||
|
||||
expect(screen.getByText("2 of 2 tools enabled for user access")).toBeInTheDocument();
|
||||
expect(screen.getAllByText("Enabled")).toHaveLength(2);
|
||||
|
||||
fireEvent.click(screen.getByText("read_user"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onAllowedToolsChange).toHaveBeenLastCalledWith(["delete_user"]);
|
||||
});
|
||||
});
|
||||
|
||||
it("preserves a restored selection on a legacy server instead of clearing it on init", async () => {
|
||||
const onAllowedToolsChange = vi.fn();
|
||||
|
||||
render(
|
||||
<MCPToolConfiguration
|
||||
accessToken="token"
|
||||
formValues={{ url: "https://example.com/mcp", transport: "http", auth_type: "none" }}
|
||||
allowedTools={["read_user"]}
|
||||
existingAllowedTools={null}
|
||||
hasToolAllowlistInteraction
|
||||
onAllowedToolsChange={onAllowedToolsChange}
|
||||
toolNameToDisplayName={{}}
|
||||
toolNameToDescription={{}}
|
||||
onToolNameToDisplayNameChange={vi.fn()}
|
||||
onToolNameToDescriptionChange={vi.fn()}
|
||||
externalTools={tools}
|
||||
externalCanFetch
|
||||
isEditMode
|
||||
/>,
|
||||
);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(onAllowedToolsChange).toHaveBeenCalled();
|
||||
});
|
||||
expect(onAllowedToolsChange).toHaveBeenLastCalledWith(["read_user"]);
|
||||
expect(onAllowedToolsChange).not.toHaveBeenCalledWith([]);
|
||||
});
|
||||
|
||||
it("keeps legacy unrestricted tools disabled after all are toggled off", async () => {
|
||||
const Wrapper = () => {
|
||||
const [allowedTools, setAllowedTools] = useState<string[]>([]);
|
||||
const [hasToolAllowlistInteraction, setHasToolAllowlistInteraction] = useState(false);
|
||||
|
||||
return (
|
||||
<MCPToolConfiguration
|
||||
accessToken="token"
|
||||
formValues={{ url: "https://example.com/mcp", transport: "http", auth_type: "none" }}
|
||||
allowedTools={allowedTools}
|
||||
existingAllowedTools={null}
|
||||
hasToolAllowlistInteraction={hasToolAllowlistInteraction}
|
||||
onToolAllowlistInteraction={() => setHasToolAllowlistInteraction(true)}
|
||||
onAllowedToolsChange={setAllowedTools}
|
||||
toolNameToDisplayName={{}}
|
||||
toolNameToDescription={{}}
|
||||
onToolNameToDisplayNameChange={vi.fn()}
|
||||
onToolNameToDescriptionChange={vi.fn()}
|
||||
externalTools={tools}
|
||||
externalCanFetch
|
||||
isEditMode
|
||||
/>
|
||||
);
|
||||
};
|
||||
|
||||
render(<Wrapper />);
|
||||
|
||||
fireEvent.click(screen.getByText("Flat List"));
|
||||
fireEvent.click(screen.getByText("read_user"));
|
||||
fireEvent.click(screen.getByText("delete_user"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByText("0 of 2 tools enabled for user access")).toBeInTheDocument();
|
||||
expect(screen.getAllByText("Disabled")).toHaveLength(2);
|
||||
});
|
||||
});
|
||||
});
|
||||
@ -21,6 +21,8 @@ interface MCPToolConfigurationProps {
|
||||
toolNameToDescription: Record<string, string>;
|
||||
onToolNameToDisplayNameChange: (map: Record<string, string>) => void;
|
||||
onToolNameToDescriptionChange: (map: Record<string, string>) => void;
|
||||
hasToolAllowlistInteraction?: boolean;
|
||||
onToolAllowlistInteraction?: () => void;
|
||||
/** Curated key tools from the OpenAPI registry preset (shown before spec loads). */
|
||||
keyTools?: KeyTool[];
|
||||
/** External tool state lifted from parent to avoid duplicate fetch requests. */
|
||||
@ -28,6 +30,8 @@ interface MCPToolConfigurationProps {
|
||||
externalIsLoading?: boolean;
|
||||
externalError?: string | null;
|
||||
externalCanFetch?: boolean;
|
||||
/** When true, do not auto-select all tools for servers with no stored allowlist. */
|
||||
isEditMode?: boolean;
|
||||
}
|
||||
|
||||
interface ToolEntry {
|
||||
@ -70,9 +74,7 @@ const ToolRow: React.FC<ToolRowProps> = ({
|
||||
<Checkbox checked={isEnabled} onChange={() => onToggle(tool.name)} />
|
||||
<div className="flex-1">
|
||||
<div className="flex items-center gap-2">
|
||||
<Text className="font-medium text-gray-900">
|
||||
{toolNameToDisplayName[tool.name] || tool.name}
|
||||
</Text>
|
||||
<Text className="font-medium text-gray-900">{toolNameToDisplayName[tool.name] || tool.name}</Text>
|
||||
<span
|
||||
className={`px-2 py-0.5 text-xs rounded-full font-medium ${
|
||||
isEnabled ? "bg-green-100 text-green-800" : "bg-red-100 text-red-800"
|
||||
@ -99,9 +101,7 @@ const ToolRow: React.FC<ToolRowProps> = ({
|
||||
type="button"
|
||||
onClick={(e) => onToggleExpand(tool.name, e)}
|
||||
className={`p-1.5 rounded-md transition-colors ${
|
||||
isEditExpanded
|
||||
? "bg-blue-100 text-blue-600"
|
||||
: "text-gray-400 hover:text-gray-600 hover:bg-gray-100"
|
||||
isEditExpanded ? "bg-blue-100 text-blue-600" : "text-gray-400 hover:text-gray-600 hover:bg-gray-100"
|
||||
}`}
|
||||
title="Edit display name and description"
|
||||
>
|
||||
@ -153,11 +153,14 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
toolNameToDescription,
|
||||
onToolNameToDisplayNameChange,
|
||||
onToolNameToDescriptionChange,
|
||||
hasToolAllowlistInteraction = false,
|
||||
onToolAllowlistInteraction,
|
||||
keyTools,
|
||||
externalTools,
|
||||
externalIsLoading,
|
||||
externalError,
|
||||
externalCanFetch,
|
||||
isEditMode = false,
|
||||
}) => {
|
||||
const previousToolsRef = useRef<ToolEntry[]>([]);
|
||||
const [toolSearchTerm, setToolSearchTerm] = useState("");
|
||||
@ -176,9 +179,9 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
enabled: !hasExternalState,
|
||||
});
|
||||
const tools: ToolEntry[] = hasExternalState ? externalTools : internalHook.tools;
|
||||
const isLoadingTools = hasExternalState ? (externalIsLoading ?? false) : internalHook.isLoadingTools;
|
||||
const toolsError = hasExternalState ? (externalError ?? null) : internalHook.toolsError;
|
||||
const canFetchTools = hasExternalState ? (externalCanFetch ?? false) : internalHook.canFetchTools;
|
||||
const isLoadingTools = hasExternalState ? externalIsLoading ?? false : internalHook.isLoadingTools;
|
||||
const toolsError = hasExternalState ? externalError ?? null : internalHook.toolsError;
|
||||
const canFetchTools = hasExternalState ? externalCanFetch ?? false : internalHook.canFetchTools;
|
||||
|
||||
// Fuzzy-match curated key tool names against actual loaded tool names
|
||||
const suggestedTools = useMemo(() => {
|
||||
@ -186,7 +189,10 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
const usedNames = new Set<string>();
|
||||
const result: typeof tools = [];
|
||||
for (const keyTool of keyTools) {
|
||||
const keywords = keyTool.name.split("_").map((k) => k.toLowerCase()).filter((k) => k.length > 1);
|
||||
const keywords = keyTool.name
|
||||
.split("_")
|
||||
.map((k) => k.toLowerCase())
|
||||
.filter((k) => k.length > 1);
|
||||
if (keywords.length === 0) continue;
|
||||
const normalize = (s: string) => s.toLowerCase().replace(/[-_/]/g, " ");
|
||||
let match = tools.find((t) => {
|
||||
@ -209,10 +215,7 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
return result;
|
||||
}, [keyTools, tools]);
|
||||
|
||||
const suggestedToolNames = useMemo(
|
||||
() => new Set(suggestedTools.map((t) => t.name)),
|
||||
[suggestedTools]
|
||||
);
|
||||
const suggestedToolNames = useMemo(() => new Set(suggestedTools.map((t) => t.name)), [suggestedTools]);
|
||||
|
||||
// Filter tools based on search term
|
||||
const filteredTools = useMemo(
|
||||
@ -224,27 +227,36 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
(tool.description && tool.description.toLowerCase().includes(searchLower))
|
||||
);
|
||||
}),
|
||||
[tools, toolSearchTerm]
|
||||
[tools, toolSearchTerm],
|
||||
);
|
||||
|
||||
const pinnedFiltered = useMemo(
|
||||
() => filteredTools.filter((t) => suggestedToolNames.has(t.name)),
|
||||
[filteredTools, suggestedToolNames]
|
||||
[filteredTools, suggestedToolNames],
|
||||
);
|
||||
|
||||
const restFiltered = useMemo(
|
||||
() => filteredTools.filter((t) => !suggestedToolNames.has(t.name)),
|
||||
[filteredTools, suggestedToolNames]
|
||||
[filteredTools, suggestedToolNames],
|
||||
);
|
||||
|
||||
// Auto-select tools when tools are first loaded or when tools list changes
|
||||
useEffect(() => {
|
||||
const currentToolNames = tools.map((tool) => tool.name).sort().join(",");
|
||||
const previousToolNames = previousToolsRef.current.map((tool) => tool.name).sort().join(",");
|
||||
const currentToolNames = tools
|
||||
.map((tool) => tool.name)
|
||||
.sort()
|
||||
.join(",");
|
||||
const previousToolNames = previousToolsRef.current
|
||||
.map((tool) => tool.name)
|
||||
.sort()
|
||||
.join(",");
|
||||
const toolsListChanged = currentToolNames !== previousToolNames;
|
||||
|
||||
// Reset initialization when a new preset is selected (suggestedTools fingerprint changes)
|
||||
const currentSuggestedNames = suggestedTools.map((t) => t.name).sort().join(",");
|
||||
const currentSuggestedNames = suggestedTools
|
||||
.map((t) => t.name)
|
||||
.sort()
|
||||
.join(",");
|
||||
if (currentSuggestedNames !== previousSuggestedToolNamesRef.current) {
|
||||
previousSuggestedToolNamesRef.current = currentSuggestedNames;
|
||||
if (currentSuggestedNames !== "") {
|
||||
@ -258,17 +270,19 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
if (!hasInitializedRef.current) {
|
||||
hasInitializedRef.current = true;
|
||||
|
||||
if (existingAllowedTools && existingAllowedTools.length > 0) {
|
||||
// Edit mode: pre-select tools that match existing allowed tools
|
||||
const validExistingTools = existingAllowedTools.filter((toolName) =>
|
||||
availableToolNames.includes(toolName)
|
||||
);
|
||||
if (existingAllowedTools !== null) {
|
||||
// Edit mode: honor stored allowlist, including [] (user cleared all tools).
|
||||
const validExistingTools = existingAllowedTools.filter((toolName) => availableToolNames.includes(toolName));
|
||||
onAllowedToolsChange(validExistingTools);
|
||||
} else if (isEditMode) {
|
||||
// Unrestricted legacy server: preserve a restored/in-progress
|
||||
// selection, otherwise do not auto-select before the user picks tools.
|
||||
onAllowedToolsChange(
|
||||
hasToolAllowlistInteraction ? allowedTools.filter((toolName) => availableToolNames.includes(toolName)) : [],
|
||||
);
|
||||
} else if (suggestedTools.length > 0) {
|
||||
// OpenAPI preset: only enable suggested tools by default
|
||||
onAllowedToolsChange(
|
||||
suggestedTools.map((t) => t.name).filter((name) => availableToolNames.includes(name))
|
||||
);
|
||||
onAllowedToolsChange(suggestedTools.map((t) => t.name).filter((name) => availableToolNames.includes(name)));
|
||||
} else {
|
||||
// Create mode: auto-select all tools
|
||||
onAllowedToolsChange(availableToolNames);
|
||||
@ -281,13 +295,34 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
}
|
||||
|
||||
previousToolsRef.current = tools;
|
||||
}, [tools, allowedTools, existingAllowedTools, onAllowedToolsChange, suggestedTools]);
|
||||
}, [
|
||||
tools,
|
||||
allowedTools,
|
||||
existingAllowedTools,
|
||||
onAllowedToolsChange,
|
||||
suggestedTools,
|
||||
hasToolAllowlistInteraction,
|
||||
isEditMode,
|
||||
]);
|
||||
|
||||
const isLegacyUnrestrictedEdit =
|
||||
isEditMode && existingAllowedTools === null && allowedTools.length === 0 && !hasToolAllowlistInteraction;
|
||||
const effectiveAllowedTools = useMemo(
|
||||
() => (isLegacyUnrestrictedEdit ? tools.map((tool) => tool.name) : allowedTools),
|
||||
[allowedTools, isLegacyUnrestrictedEdit, tools],
|
||||
);
|
||||
const effectiveAllowedToolNames = useMemo(() => new Set(effectiveAllowedTools), [effectiveAllowedTools]);
|
||||
|
||||
const handleAllowedToolsChange = (nextAllowedTools: string[]) => {
|
||||
onToolAllowlistInteraction?.();
|
||||
onAllowedToolsChange(nextAllowedTools);
|
||||
};
|
||||
|
||||
const handleToolToggle = (toolName: string) => {
|
||||
if (allowedTools.includes(toolName)) {
|
||||
onAllowedToolsChange(allowedTools.filter((name) => name !== toolName));
|
||||
if (effectiveAllowedToolNames.has(toolName)) {
|
||||
handleAllowedToolsChange(effectiveAllowedTools.filter((name) => name !== toolName));
|
||||
} else {
|
||||
onAllowedToolsChange([...allowedTools, toolName]);
|
||||
handleAllowedToolsChange([...effectiveAllowedTools, toolName]);
|
||||
}
|
||||
};
|
||||
|
||||
@ -327,25 +362,27 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
const handleEnableSuggested = () => {
|
||||
// Enable ALL suggested tools (not just the currently filtered subset)
|
||||
const suggestedNames = suggestedTools.map((t) => t.name);
|
||||
const others = allowedTools.filter((n) => !suggestedToolNames.has(n));
|
||||
onAllowedToolsChange([...others, ...suggestedNames]);
|
||||
const missingSuggestedNames = suggestedNames.filter((name) => !effectiveAllowedToolNames.has(name));
|
||||
if (missingSuggestedNames.length === 0) return;
|
||||
handleAllowedToolsChange([...effectiveAllowedTools, ...missingSuggestedNames]);
|
||||
};
|
||||
|
||||
const handleDisableSuggested = () => {
|
||||
// Disable ALL suggested tools (not just the currently filtered subset)
|
||||
onAllowedToolsChange(allowedTools.filter((n) => !suggestedToolNames.has(n)));
|
||||
handleAllowedToolsChange(effectiveAllowedTools.filter((n) => !suggestedToolNames.has(n)));
|
||||
};
|
||||
|
||||
const handleEnableRest = () => {
|
||||
// Enable ALL non-suggested tools (not just the currently filtered subset)
|
||||
const restNames = tools.filter((t) => !suggestedToolNames.has(t.name)).map((t) => t.name);
|
||||
const current = new Set(allowedTools);
|
||||
onAllowedToolsChange([...allowedTools, ...restNames.filter((n) => !current.has(n))]);
|
||||
const missingRestNames = restNames.filter((n) => !effectiveAllowedToolNames.has(n));
|
||||
if (missingRestNames.length === 0) return;
|
||||
handleAllowedToolsChange([...effectiveAllowedTools, ...missingRestNames]);
|
||||
};
|
||||
|
||||
const handleDisableRest = () => {
|
||||
// Disable ALL non-suggested tools (not just the currently filtered subset)
|
||||
onAllowedToolsChange(allowedTools.filter((n) => suggestedToolNames.has(n)));
|
||||
handleAllowedToolsChange(effectiveAllowedTools.filter((n) => suggestedToolNames.has(n)));
|
||||
};
|
||||
|
||||
// Don't show anything if required fields aren't filled
|
||||
@ -411,14 +448,15 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
)}
|
||||
|
||||
{/* No tools state */}
|
||||
{!isLoadingTools && !toolsError && tools.length === 0 && canFetchTools && (
|
||||
keyTools && keyTools.length > 0 ? (
|
||||
{!isLoadingTools &&
|
||||
!toolsError &&
|
||||
tools.length === 0 &&
|
||||
canFetchTools &&
|
||||
(keyTools && keyTools.length > 0 ? (
|
||||
<div className="text-center py-4 text-gray-400 border rounded-lg border-dashed">
|
||||
<ToolOutlined className="text-2xl mb-2" />
|
||||
<Text>No tools loaded from spec</Text>
|
||||
<Text className="text-sm block mt-1">
|
||||
Expected tools: {keyTools.map((t) => t.name).join(", ")}
|
||||
</Text>
|
||||
<Text className="text-sm block mt-1">Expected tools: {keyTools.map((t) => t.name).join(", ")}</Text>
|
||||
</div>
|
||||
) : (
|
||||
<div className="text-center py-6 text-gray-400 border rounded-lg border-dashed">
|
||||
@ -427,8 +465,7 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
<br />
|
||||
<Text className="text-sm">Connect to an MCP server with tools to configure them</Text>
|
||||
</div>
|
||||
)
|
||||
)}
|
||||
))}
|
||||
|
||||
{/* Incomplete form state */}
|
||||
{!canFetchTools && (formValues.url || formValues.spec_path) && (
|
||||
@ -446,8 +483,8 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
<div className="flex items-center gap-2 p-3 bg-green-50 rounded-lg border border-green-200">
|
||||
<CheckCircleOutlined className="text-green-600" />
|
||||
<Text className="text-green-700 font-medium">
|
||||
{allowedTools.length} of {tools.length} {tools.length === 1 ? "tool" : "tools"} enabled for user
|
||||
access
|
||||
{effectiveAllowedTools.length} of {tools.length} {tools.length === 1 ? "tool" : "tools"} enabled for
|
||||
user access
|
||||
</Text>
|
||||
</div>
|
||||
|
||||
@ -467,8 +504,8 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
<McpCrudPermissionPanel
|
||||
tools={tools}
|
||||
searchFilter={toolSearchTerm}
|
||||
value={allowedTools.length === 0 ? undefined : allowedTools}
|
||||
onChange={(allowed) => onAllowedToolsChange(allowed)}
|
||||
value={isLegacyUnrestrictedEdit ? undefined : allowedTools}
|
||||
onChange={handleAllowedToolsChange}
|
||||
/>
|
||||
)}
|
||||
|
||||
@ -482,84 +519,82 @@ const MCPToolConfiguration: React.FC<MCPToolConfigurationProps> = ({
|
||||
</div>
|
||||
) : (
|
||||
<div className="space-y-2">
|
||||
{pinnedFiltered.length > 0 && (
|
||||
<>
|
||||
<div className="flex items-center justify-between px-1">
|
||||
<p className="text-xs font-semibold text-gray-500 uppercase tracking-wide">
|
||||
Suggested tools
|
||||
</p>
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleEnableSuggested}
|
||||
className="text-xs text-blue-600 hover:text-blue-700"
|
||||
>
|
||||
Enable all
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleDisableSuggested}
|
||||
className="text-xs text-gray-500 hover:text-gray-700"
|
||||
>
|
||||
Disable all
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
{pinnedFiltered.map((tool) => (
|
||||
<ToolRow
|
||||
key={tool.name}
|
||||
tool={tool}
|
||||
isEnabled={allowedTools.includes(tool.name)}
|
||||
isEditExpanded={expandedTools.has(tool.name)}
|
||||
toolNameToDisplayName={toolNameToDisplayName}
|
||||
toolNameToDescription={toolNameToDescription}
|
||||
onToggle={handleToolToggle}
|
||||
onToggleExpand={handleToggleEditExpanded}
|
||||
onDisplayNameChange={handleDisplayNameChange}
|
||||
onDescriptionChange={handleDescriptionChange}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
{restFiltered.length > 0 && (
|
||||
<>
|
||||
<div className="flex items-center justify-between px-1 pt-2">
|
||||
<p className="text-xs font-semibold text-gray-500 uppercase tracking-wide">
|
||||
{pinnedFiltered.length > 0 ? "All tools" : "Tools"}
|
||||
</p>
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleEnableRest}
|
||||
className="text-xs text-blue-600 hover:text-blue-700"
|
||||
>
|
||||
Enable all
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleDisableRest}
|
||||
className="text-xs text-gray-500 hover:text-gray-700"
|
||||
>
|
||||
Disable all
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
{restFiltered.map((tool) => (
|
||||
<ToolRow
|
||||
key={tool.name}
|
||||
tool={tool}
|
||||
isEnabled={allowedTools.includes(tool.name)}
|
||||
isEditExpanded={expandedTools.has(tool.name)}
|
||||
toolNameToDisplayName={toolNameToDisplayName}
|
||||
toolNameToDescription={toolNameToDescription}
|
||||
onToggle={handleToolToggle}
|
||||
onToggleExpand={handleToggleEditExpanded}
|
||||
onDisplayNameChange={handleDisplayNameChange}
|
||||
onDescriptionChange={handleDescriptionChange}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
{pinnedFiltered.length > 0 && (
|
||||
<>
|
||||
<div className="flex items-center justify-between px-1">
|
||||
<p className="text-xs font-semibold text-gray-500 uppercase tracking-wide">Suggested tools</p>
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleEnableSuggested}
|
||||
className="text-xs text-blue-600 hover:text-blue-700"
|
||||
>
|
||||
Enable all
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleDisableSuggested}
|
||||
className="text-xs text-gray-500 hover:text-gray-700"
|
||||
>
|
||||
Disable all
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
{pinnedFiltered.map((tool) => (
|
||||
<ToolRow
|
||||
key={tool.name}
|
||||
tool={tool}
|
||||
isEnabled={effectiveAllowedToolNames.has(tool.name)}
|
||||
isEditExpanded={expandedTools.has(tool.name)}
|
||||
toolNameToDisplayName={toolNameToDisplayName}
|
||||
toolNameToDescription={toolNameToDescription}
|
||||
onToggle={handleToolToggle}
|
||||
onToggleExpand={handleToggleEditExpanded}
|
||||
onDisplayNameChange={handleDisplayNameChange}
|
||||
onDescriptionChange={handleDescriptionChange}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
{restFiltered.length > 0 && (
|
||||
<>
|
||||
<div className="flex items-center justify-between px-1 pt-2">
|
||||
<p className="text-xs font-semibold text-gray-500 uppercase tracking-wide">
|
||||
{pinnedFiltered.length > 0 ? "All tools" : "Tools"}
|
||||
</p>
|
||||
<div className="flex gap-2">
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleEnableRest}
|
||||
className="text-xs text-blue-600 hover:text-blue-700"
|
||||
>
|
||||
Enable all
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={handleDisableRest}
|
||||
className="text-xs text-gray-500 hover:text-gray-700"
|
||||
>
|
||||
Disable all
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
{restFiltered.map((tool) => (
|
||||
<ToolRow
|
||||
key={tool.name}
|
||||
tool={tool}
|
||||
isEnabled={effectiveAllowedToolNames.has(tool.name)}
|
||||
isEditExpanded={expandedTools.has(tool.name)}
|
||||
toolNameToDisplayName={toolNameToDisplayName}
|
||||
toolNameToDescription={toolNameToDescription}
|
||||
onToggle={handleToolToggle}
|
||||
onToggleExpand={handleToggleEditExpanded}
|
||||
onDisplayNameChange={handleDisplayNameChange}
|
||||
onDescriptionChange={handleDescriptionChange}
|
||||
/>
|
||||
))}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
|
||||
@ -105,6 +105,7 @@ export interface MCPInfo {
|
||||
description?: string;
|
||||
logo_url?: string;
|
||||
mcp_server_cost_info?: MCPServerCostInfo | null;
|
||||
tool_allowlist_enforced?: boolean;
|
||||
}
|
||||
|
||||
// Define the structure for a single MCP tool
|
||||
|
||||
Loading…
Reference in New Issue
Block a user