Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion litellm/types/mcp.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class MCPPublicServer(BaseModel):
name: str
alias: Optional[str] = None
server_name: Optional[str] = None
url: Optional[str] = None
transport: MCPTransportType
spec_path: Optional[str] = None
auth_type: Optional[MCPAuthType] = None
Expand Down
41 changes: 41 additions & 0 deletions tests/test_litellm/proxy/public_endpoints/test_public_endpoints.py
Original file line number Diff line number Diff line change
Expand Up @@ -819,3 +819,44 @@ def test_public_mcp_hub_returns_empty_when_whitelist_unset():
assert response.status_code == 200
assert response.json() == []
app.dependency_overrides.clear()


def test_public_mcp_hub_does_not_expose_upstream_url():
"""Regression: /public/mcp_hub is unauthenticated, so the gateway-internal
upstream url must never appear in its response even when the server has one."""
from litellm.types.mcp_server.mcp_server_manager import MCPServer
from litellm.proxy._types import MCPTransport

app = FastAPI()
app.include_router(router)
app.dependency_overrides[user_api_key_auth] = lambda: MagicMock()
client = TestClient(app)

secret_url = "https://internal-only.example.com/mcp"
server = MCPServer(
server_id="listed",
name="listed",
server_name="listed",
url=secret_url,
transport=MCPTransport.http,
available_on_public_internet=True,
)

mock_manager = MagicMock()
mock_manager.get_public_mcp_servers.return_value = [server]

with (
patch("litellm.public_mcp_servers", ["listed"]),
patch(
"litellm.proxy._experimental.mcp_server.mcp_server_manager.global_mcp_server_manager",
mock_manager,
),
):
response = client.get("/public/mcp_hub")

assert response.status_code == 200
data = response.json()
assert [item["server_id"] for item in data] == ["listed"]
assert all("url" not in item for item in data)
assert secret_url not in response.text
app.dependency_overrides.clear()
10 changes: 0 additions & 10 deletions ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@
supported_openai_params?: string[];
is_public_model_group: boolean;
// Allow any additional properties for flexibility
[key: string]: any;

Check warning on line 60 in ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
}

const ModelHubTable: React.FC<ModelHubTableProps> = ({ accessToken, publicPage, premiumUser, userRole }) => {

Check warning on line 63 in ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Arrow function has a complexity of 83. Maximum allowed is 20
// Admin Viewer follows the read-parity rule: see the AI Hub catalog, but
// cannot toggle public visibility (write).
const canModify = isProxyAdminRole(userRole || "");
Expand Down Expand Up @@ -177,7 +177,7 @@
const response = await getAgentsList(accessToken);
console.log("AgentHubData:", response);
let agents = response.agents;
let agent_card_list = agents.map((agent: any) => ({

Check warning on line 180 in ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
agent_id: agent.agent_id,
...agent.agent_card_params,
is_public: agent.litellm_params.is_public,
Expand Down Expand Up @@ -351,7 +351,7 @@
try {
const response = await getAgentsList(accessToken);
let agents = response.agents;
let agent_card_list = agents.map((agent: any) => ({

Check warning on line 354 in ui/litellm-dashboard/src/components/AIHub/ModelHubTable.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
agent_id: agent.agent_id,
...agent.agent_card_params,
is_public: agent.is_public,
Expand Down Expand Up @@ -935,16 +935,6 @@
<div>
<Text className="text-lg font-semibold mb-4">Connection Details</Text>
<div className="space-y-2">
<div>
<Text className="font-medium">URL:</Text>
<div className="flex items-center space-x-2 mt-1">
<Text className="text-sm break-all bg-gray-100 p-2 rounded flex-1">{selectedMcpServer.url}</Text>
<CopyOutlined
onClick={() => copyToClipboard(selectedMcpServer.url)}
className="cursor-pointer text-gray-500 hover:text-blue-500 flex-shrink-0"
/>
</div>
</div>
{selectedMcpServer.command && (
<div>
<Text className="font-medium">Command:</Text>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { render, screen } from "@testing-library/react";
import { vi } from "vitest";
import { flexRender, getCoreRowModel, useReactTable } from "@tanstack/react-table";
import { mcpHubColumns, MCPServerData } from "./mcp_hub_table_columns";

const SERVER_URL = "https://mcp.exa.ai/mcp";

const mockServer: MCPServerData = {
server_id: "server-1",
server_name: "exa_test",
description: "Fast, intelligent web search and web crawling",
url: SERVER_URL,
transport: "http",
auth_type: "none",
created_at: "2026-01-01T00:00:00Z",
created_by: "admin",
updated_at: "2026-01-01T00:00:00Z",
updated_by: "admin",
teams: [],
mcp_access_groups: [],
allowed_tools: [],
extra_headers: [],
mcp_info: {},
static_headers: {},
status: "active",
args: [],
env: {},
};

function TestTable({ data }: { data: MCPServerData[] }) {
const columns = mcpHubColumns(vi.fn(), vi.fn(), false);
const table = useReactTable({ data, columns, getCoreRowModel: getCoreRowModel() });

return (
<table>
<thead>
{table.getHeaderGroups().map((hg) => (
<tr key={hg.id}>
{hg.headers.map((h) => (
<th key={h.id}>{flexRender(h.column.columnDef.header, h.getContext())}</th>
))}
</tr>
))}
</thead>
<tbody>
{table.getRowModel().rows.map((row) => (
<tr key={row.id}>
{row.getVisibleCells().map((cell) => (
<td key={cell.id}>{flexRender(cell.column.columnDef.cell, cell.getContext())}</td>
))}
</tr>
))}
</tbody>
</table>
);
}

describe("mcpHubColumns", () => {
it("renders the server row", () => {
render(<TestTable data={[mockServer]} />);
expect(screen.getByText("exa_test")).toBeInTheDocument();
});

it("keeps the non-sensitive columns", () => {
render(<TestTable data={[mockServer]} />);
expect(screen.getByText("Server Name")).toBeInTheDocument();
expect(screen.getByText("Transport")).toBeInTheDocument();
expect(screen.getByText("Auth Type")).toBeInTheDocument();
});

it("does not expose a URL column header", () => {
render(<TestTable data={[mockServer]} />);
expect(screen.queryByText("URL")).not.toBeInTheDocument();
expect(mcpHubColumns(vi.fn(), vi.fn(), false).some((c) => c.header === "URL")).toBe(false);
Comment on lines +71 to +74

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Header-string check can be silently bypassed

The assertion c.header === "URL" only matches a column whose header property is the exact string "URL". If someone re-adds the column with a header like "Server URL", a React node, or even a function, the check passes even though the URL is rendered. The companion queryByText(SERVER_URL) test at line 79 catches rendered output and is the stronger guard; consider removing the brittle static check or complementing it with an accessorKey assertion (e.g., c.accessorKey !== "url") so the test actually fails if the URL data is wired back up regardless of the label used.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

});

it("does not render the server url anywhere in the table", () => {
render(<TestTable data={[mockServer]} />);
expect(screen.queryByText(SERVER_URL)).not.toBeInTheDocument();
});
});
Comment on lines +57 to +81

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Test coverage gap for public hub and Detail modals

The new test suite only exercises mcpHubColumns from mcp_hub_table_columns.tsx. The PR makes three additional removals — the URL column in publicMCPHubColumns (public_model_hub.tsx ~line 925) and the URL block in both Detail modals (ModelHubTable.tsx ~line 935 and public_model_hub.tsx ~line 1870) — none of which have an automated regression guard. A future refactor that restores any of these surfaces would not be caught until manual review.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment on lines +58 to +81

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 No parallel test coverage for publicMCPHubColumns in public_model_hub.tsx

The new test file only validates mcpHubColumns (the authenticated hub). The public hub has its own, separately-defined column set (publicMCPHubColumns) whose URL column was also removed in this PR. That parallel definition has no regression test, so a future accidental re-addition of the URL column there would go undetected. Adding a second describe block that imports and exercises publicMCPHubColumns (or the public hub component) in the same style would close this gap.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

24 changes: 0 additions & 24 deletions ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
url: string;
transport: string;
auth_type: string;
credentials?: any;

Check warning on line 14 in ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
created_at: string;
created_by: string;
updated_at: string;
Expand All @@ -19,16 +19,16 @@
teams: string[];
mcp_access_groups: string[];
allowed_tools: string[];
extra_headers: any[];

Check warning on line 22 in ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
mcp_info: Record<string, any>;

Check warning on line 23 in ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
static_headers: Record<string, any>;

Check warning on line 24 in ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
status: string;
last_health_check?: string | null;
health_check_error?: string | null;
command?: string | null;
args: string[];
env: Record<string, any>;

Check warning on line 30 in ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
[key: string]: any;

Check warning on line 31 in ui/litellm-dashboard/src/components/mcp_hub_table_columns.tsx

View workflow job for this annotation

GitHub Actions / frontend-lint

Unexpected any. Specify a different type
}

export const mcpHubColumns = (
Expand Down Expand Up @@ -78,30 +78,6 @@
className: "hidden md:table-cell",
},
},
{
header: "URL",
accessorKey: "url",
enableSorting: true,
sortingFn: "alphanumeric",
cell: ({ row }) => {
const server = row.original;

return (
<div className="flex items-center space-x-2">
<Text className="text-xs truncate max-w-xs">{server.url}</Text>
<Tooltip title="Copy URL">
<CopyOutlined
onClick={() => copyToClipboard(server.url)}
className="cursor-pointer text-gray-500 hover:text-blue-500 text-xs flex-shrink-0"
/>
</Tooltip>
</div>
);
},
meta: {
className: "hidden lg:table-cell",
},
},
{
header: "Transport",
accessorKey: "transport",
Expand Down
82 changes: 80 additions & 2 deletions ui/litellm-dashboard/src/components/public_model_hub.test.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { describe, it, expect, vi, beforeAll, beforeEach } from "vitest";
import { render, screen, waitFor } from "@testing-library/react";
import PublicModelHub from "./public_model_hub";
import { render, screen, waitFor, fireEvent } from "@testing-library/react";
import { flexRender, getCoreRowModel, useReactTable } from "@tanstack/react-table";
import PublicModelHub, { publicMCPHubColumns, MCPServerData } from "./public_model_hub";

vi.mock("next/navigation", () => ({
useRouter: vi.fn(() => ({
Expand Down Expand Up @@ -186,3 +187,80 @@ describe("PublicModelHub", () => {
});
});
});

const PUBLIC_SERVER_URL = "https://mcp.exa.ai/mcp";

const mockMcpServer: MCPServerData = {
server_id: "server-1",
name: "exa_test",
server_name: "exa_test",
url: PUBLIC_SERVER_URL,
transport: "http",
auth_type: "none",
mcp_info: { server_name: "exa_test", description: "Fast, intelligent web search and web crawling" },
};

function PublicMcpTestTable({ data }: { data: MCPServerData[] }) {
const columns = publicMCPHubColumns(vi.fn());
const table = useReactTable({ data, columns, getCoreRowModel: getCoreRowModel() });

return (
<table>
<thead>
{table.getHeaderGroups().map((hg) => (
<tr key={hg.id}>
{hg.headers.map((h) => (
<th key={h.id}>{flexRender(h.column.columnDef.header, h.getContext())}</th>
))}
</tr>
))}
</thead>
<tbody>
{table.getRowModel().rows.map((row) => (
<tr key={row.id}>
{row.getVisibleCells().map((cell) => (
<td key={cell.id}>{flexRender(cell.column.columnDef.cell, cell.getContext())}</td>
))}
</tr>
))}
</tbody>
</table>
);
}

describe("publicMCPHubColumns", () => {
it("keeps the non-sensitive columns", () => {
render(<PublicMcpTestTable data={[mockMcpServer]} />);
expect(screen.getByText("Server Name")).toBeInTheDocument();
expect(screen.getByText("Transport")).toBeInTheDocument();
expect(screen.getByText("Auth Type")).toBeInTheDocument();
});

it("does not expose a URL column header", () => {
render(<PublicMcpTestTable data={[mockMcpServer]} />);
expect(screen.queryByText("URL")).not.toBeInTheDocument();
expect(publicMCPHubColumns(vi.fn()).some((c) => c.header === "URL")).toBe(false);
});

it("does not render the server url anywhere in the table", () => {
render(<PublicMcpTestTable data={[mockMcpServer]} />);
expect(screen.queryByText(PUBLIC_SERVER_URL)).not.toBeInTheDocument();
});
});

describe("public hub MCP details modal", () => {
it("does not show the upstream url when a server is opened", async () => {
const networkingModule = await import("./networking");
vi.mocked(networkingModule.mcpHubPublicServersCall).mockResolvedValue([mockMcpServer]);

render(<PublicModelHub />);

fireEvent.click(await screen.findByRole("tab", { name: /MCP Hub/i }));
fireEvent.click(await screen.findByRole("button", { name: "exa_test" }));

// "Server Overview" only exists inside the opened MCP details modal,
// so finding it proves the modal rendered and the url assertion is not vacuous.
await screen.findByText("Server Overview");
expect(screen.queryByText(PUBLIC_SERVER_URL)).not.toBeInTheDocument();
});
});
Loading
Loading