scriptoriumadmin/litellm
1
0
Fork 0
mirror of https://github.com/BerriAI/litellm.git synced 2026-09-13 23:11:40 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 101

fix(mcp): warn on upstream change when the stored OAuth app is redacted from the edit form

Browse source 
The edit form's "app may not match upstream" warning only fired when a client
was sitting in the form, so a stored app (redacted to null by the GET) never
triggered it: an admin could repoint a client-forwarded server at a different
upstream and silently keep an app registered for the old one.

The backend now stamps a non-secret has_configured_client boolean on redacted
responses (derived from the credentials blob at redaction time, or from the
registry's decrypted client_id on the list path, whose table objects never
carry the blob). The non-admin and virtual-key sanitizers null it back out;
only the admin edit form needs it. The edit form fires the warning from the
flag when the credential class is unchanged (a cross-class switch replaces the
stored app, so nothing kept can mismatch), and the banner hides while the
remove-app checkbox is checked since removal writes an explicit-null credential
This commit is contained in:
Tin 2026-07-11 10:56:15 -07:00
parent 0bf81e2496
commit 559fa91304
11 changed files with 323 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
litellm/models/mcp_server.py
View file

@ -101,6 +101,12 @@ class LiteLLM_MCPServerTable(LiteLLMPydanticObjectBase):
byok_description: List[str] = Field(default_factory=list)
byok_api_key_help_url: Optional[str] = None
has_user_credential: Optional[bool] = None
has_configured_client: Optional[bool] = Field(
default=None,
description=(
"Response-only indicator that the stored (redacted) credentials include an OAuth client_id; never persisted"
),
)
source_url: Optional[str] = None
timeout: Optional[float] = None
max_concurrent_requests: Optional[int] = None

1
litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
View file

@ -4988,6 +4988,7 @@ class MCPServerManager:
is_byok=server.is_byok,
byok_description=server.byok_description,
byok_api_key_help_url=server.byok_api_key_help_url,
has_configured_client=bool(server.client_id),
source_url=server.source_url,
instructions=server.instructions,
timeout=server.timeout,

23
litellm/proxy/management_endpoints/mcp_management_endpoints.py
View file

@ -473,13 +473,31 @@ if MCP_AVAILABLE:
def _redact_mcp_credentials(
mcp_server: LiteLLM_MCPServerTable,
) -> LiteLLM_MCPServerTable:
"""Return a copy of the MCP server object with credentials removed."""
"""Return a copy of the MCP server object with credentials removed.
Stamps ``has_configured_client`` before redacting so the admin edit form
can tell that a stored OAuth app exists without ever seeing its value
(the URL-change "app may not match upstream" warning needs exactly this
bit; the stored ``client_id`` itself is encrypted and never returned).
Derives from the credentials blob when the object carries one (DB reads),
otherwise preserves a truthy flag already stamped upstream
(``_build_mcp_server_table`` on the registry list path, whose tables
never include the blob).
"""
try:
redacted_server = mcp_server.model_copy(deep=True)
except AttributeError:
redacted_server = mcp_server.copy(deep=True) # type: ignore[attr-defined]
stored_credentials = getattr(mcp_server, "credentials", None)
stored_client_id = stored_credentials.get("client_id") if isinstance(stored_credentials, dict) else None
setattr(
redacted_server,
"has_configured_client",
bool(stored_client_id or getattr(mcp_server, "has_configured_client", None)),
)
if hasattr(redacted_server, "credentials"):
setattr(redacted_server, "credentials", None)
@ -548,6 +566,8 @@ if MCP_AVAILABLE:
# admin configured. Non-admins get the per-user vars they must fill in
# from the dedicated /user-env-vars/status endpoint instead.
sanitized.env_vars = None
# Only the admin edit form needs the stored-app indicator.
sanitized.has_configured_client = None
return sanitized
def _sanitize_mcp_server_list_for_non_admin(
@ -591,6 +611,7 @@ if MCP_AVAILABLE:
sanitized.health_check_error = None
sanitized.last_health_check = None
sanitized.has_configured_client = None
sanitized.created_by = None
sanitized.updated_by = None

41
tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py
View file

@ -7335,3 +7335,44 @@ def test_build_mcp_server_table_carries_null_oauth2_flow():
table = manager._build_mcp_server_table(server)
assert table.oauth2_flow is None
def test_build_mcp_server_table_stamps_has_configured_client():
"""The list endpoint serves registry servers through this conversion WITHOUT the
credentials blob, so the redaction layer cannot see the stored client there. The
build must stamp has_configured_client from the registry's decrypted client_id or
the edit form never learns a saved OAuth app exists (its URL-change "app may not
match upstream" warning would stay silent for stored apps)."""
manager = MCPServerManager()
server = MCPServer(
server_id="stored-app-server",
name="stored_app_server",
server_name="stored_app_server",
alias="stored_app_server",
url="https://up.example.com/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.true_passthrough,
client_id="org-slack-app-client-id",
client_secret="org-slack-app-secret",
)
table = manager._build_mcp_server_table(server)
assert table.has_configured_client is True
def test_build_mcp_server_table_has_configured_client_false_without_client():
manager = MCPServerManager()
server = MCPServer(
server_id="no-app-server",
name="no_app_server",
server_name="no_app_server",
alias="no_app_server",
url="https://up.example.com/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.true_passthrough,
)
table = manager._build_mcp_server_table(server)
assert table.has_configured_client is False

115
tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py
View file

@ -5376,3 +5376,118 @@ async def test_edit_mcp_server_snapshot_failure_skips_purge_but_edit_succeeds():
assert result.server_id == server_id
mock_purge.assert_not_awaited()
def test_redact_stamps_has_configured_client_from_stored_blob():
"""The GET redacts credentials to null, so the edit form cannot see a stored OAuth
app; has_configured_client is the non-secret existence bit the URL-change "app may
not match upstream" warning keys on. Redaction must stamp it from the blob it is
about to remove, and must not leak or mutate the blob itself."""
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
_redact_mcp_credentials,
)
server = generate_mock_mcp_server_db_record()
server.credentials = {"client_id": "encrypted-client", "client_secret": "encrypted-secret"}
redacted = _redact_mcp_credentials(server)
assert redacted.has_configured_client is True
assert redacted.credentials is None
assert server.credentials == {"client_id": "encrypted-client", "client_secret": "encrypted-secret"}
@pytest.mark.parametrize(
"credentials",
[None, {"auth_value": "top-secret"}, {"client_id": ""}],
ids=["no-blob", "no-client-in-blob", "empty-client-id"],
)
def test_redact_stamps_has_configured_client_false_without_stored_client(credentials):
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
_redact_mcp_credentials,
)
server = generate_mock_mcp_server_db_record()
server.credentials = credentials
redacted = _redact_mcp_credentials(server)
assert redacted.has_configured_client is False
assert redacted.credentials is None
def test_redact_preserves_build_time_has_configured_client():
"""The list endpoint serves registry servers whose table objects never carry the
credentials blob; _build_mcp_server_table stamps the flag instead. Redaction must
preserve that stamp rather than resetting it to False for lack of a blob."""
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
_redact_mcp_credentials,
)
server = generate_mock_mcp_server_db_record()
server.credentials = None
server.has_configured_client = True
redacted = _redact_mcp_credentials(server)
assert redacted.has_configured_client is True
def test_sanitized_views_drop_has_configured_client():
"""Only the admin edit form needs the stored-app indicator; the non-admin and
virtual-key discovery views must not reveal whether an OAuth app is configured."""
import litellm.proxy.management_endpoints.mcp_management_endpoints as mgmt
server = generate_mock_mcp_server_db_record()
server.credentials = {"client_id": "encrypted-client"}
assert mgmt._sanitize_mcp_server_for_non_admin(server).has_configured_client is None
assert mgmt._sanitize_mcp_server_for_virtual_key(server).has_configured_client is None
@pytest.mark.asyncio
async def test_fetch_single_mcp_server_returns_has_configured_client():
"""End to end through GET /v1/mcp/server/{id}: a stored client surfaces only as
has_configured_client=True while the credentials stay redacted."""
mock_server = generate_mock_mcp_server_db_record(server_id="server-1", alias="Server 1")
mock_server.credentials = {"client_id": "encrypted-client", "client_secret": "encrypted-secret"}
mock_prisma_client = MagicMock()
mock_health_result = generate_mock_mcp_server_db_record(server_id="server-1", alias="Server 1")
mock_health_result.status = "healthy"
mock_health_result.last_health_check = datetime.now()
mock_health_result.health_check_error = None
mock_user_auth = generate_mock_user_api_key_auth(user_role=LitellmUserRoles.PROXY_ADMIN)
with (
patch(
"litellm.proxy.management_endpoints.mcp_management_endpoints.get_prisma_client_or_throw",
return_value=mock_prisma_client,
),
patch(
"litellm.proxy.management_endpoints.mcp_management_endpoints.get_mcp_server",
AsyncMock(return_value=mock_server),
),
patch(
"litellm.proxy.management_endpoints.mcp_management_endpoints.global_mcp_server_manager.health_check_server",
AsyncMock(return_value=mock_health_result),
),
patch(
"litellm.proxy.management_endpoints.mcp_management_endpoints._user_has_admin_view",
return_value=True,
),
):
from litellm.proxy.management_endpoints.mcp_management_endpoints import (
fetch_mcp_server,
)
result = await fetch_mcp_server(
request=_make_mock_request(),
server_id="server-1",
user_api_key_dict=mock_user_auth,
)
assert result.has_configured_client is True
assert result.credentials is None

17
ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.test.tsx
View file

@ -51,4 +51,21 @@ describe("PassthroughAuthorizeSection credential-class-aware copy", () => {
);
expect(screen.getByText(/registered for the previous upstream/)).toBeInTheDocument();
});
it("hides the keep+warn banner while the remove-stored-app checkbox is checked", () => {
render(
<WithForm>
<PassthroughAuthorizeSection
authType="true_passthrough"
oauthFlow={noopFlow}
isEditing
savedAuthType="true_passthrough"
appMayNotMatchUpstream
removeStoredApp
onRemoveStoredAppChange={() => {}}
/>
</WithForm>,
);
expect(screen.queryByText(/registered for the previous upstream/)).not.toBeInTheDocument();
});
});

7
ui/litellm-dashboard/src/components/mcp_tools/PassthroughAuthorizeSection.tsx
View file

@ -75,10 +75,11 @@ export default function PassthroughAuthorizeSection({
and is never saved to LiteLLM. An OAuth app configured below IS saved with the server, so internal users who
authorize from the Tools page go through it.
</p>
{appMayNotMatchUpstream && (
{appMayNotMatchUpstream && !removeStoredApp && (
<p className="text-sm text-amber-600">
You changed the upstream URL or endpoints; the OAuth app entered here was registered for the previous upstream
and may not be valid. Update the client ID, or clear it to use dynamic client registration.
You changed the upstream URL or endpoints; the OAuth app configured for this server was registered for the
previous upstream and may not be valid. Enter a client ID registered for the new upstream, or remove the app
to use dynamic client registration.
</p>
)}
<Form.Item

4
ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.test.tsx
View file

@ -802,7 +802,9 @@ describe("CreateMCPServer", () => {
});
// Keep + warn: the app stays in the field, and a non-blocking warning appears.
expect(screen.getByText(/OAuth app entered here was registered for the previous upstream/)).toBeInTheDocument();
expect(
screen.getByText(/OAuth app configured for this server was registered for the previous upstream/),
).toBeInTheDocument();
});
it("keeps client_secret when only client_id is edited after a client-forwarded authorize", async () => {

97
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.test.tsx
View file

@ -1549,6 +1549,103 @@ describe("MCPServerEdit (OAuth token persistence on save)", () => {
expect(screen.getByText(/registered for the previous upstream/)).toBeInTheDocument();
});
it("warns after a URL change when the stored app is redacted (has_configured_client, blank fields)", async () => {
// The real GET redacts credentials to null, so the form holds no client even though the server
// has a saved app. has_configured_client is the backend's non-secret "a client exists" bit; the
// warning must fire from it, otherwise keep-existing silently keeps an app registered for the
// old upstream and the admin is never told.
render(
<MCPServerEdit
mcpServer={{
...interactiveOAuthServer,
auth_type: "true_passthrough",
credentials: null,
has_configured_client: true,
}}
accessToken="access-token"
userID="user-1"
onCancel={vi.fn()}
onSuccess={vi.fn()}
availableAccessGroups={[]}
/>,
);
expect(screen.queryByText(/registered for the previous upstream/)).not.toBeInTheDocument();
await act(async () => {
fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), {
target: { value: "https://different.example.com/mcp" },
});
});
expect(screen.getByText(/registered for the previous upstream/)).toBeInTheDocument();
});
it("hides the stored-app warning while the remove checkbox is checked and restores it on uncheck", async () => {
// Removal writes an explicit-null credential on save, so nothing kept can mismatch; unchecking
// returns to keep-existing, where the mismatch concern is live again.
render(
<MCPServerEdit
mcpServer={{
...interactiveOAuthServer,
auth_type: "true_passthrough",
credentials: null,
has_configured_client: true,
}}
accessToken="access-token"
userID="user-1"
onCancel={vi.fn()}
onSuccess={vi.fn()}
availableAccessGroups={[]}
/>,
);
await act(async () => {
fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), {
target: { value: "https://different.example.com/mcp" },
});
});
expect(screen.getByText(/registered for the previous upstream/)).toBeInTheDocument();
const removeCheckbox = screen.getByRole("checkbox", { name: /Remove the saved OAuth app on save/ });
fireEvent.click(removeCheckbox);
expect(screen.queryByText(/registered for the previous upstream/)).not.toBeInTheDocument();
fireEvent.click(removeCheckbox);
expect(screen.getByText(/registered for the previous upstream/)).toBeInTheDocument();
});
it("does not warn from has_configured_client after a cross-class auth switch", async () => {
// Saved oauth2 server with a stored client (e.g. a persisted DCR app). Switching to
// true_passthrough is a cross-class change: blanks mean "no app" and the stored app is replaced
// on save, so a URL change has nothing kept to warn about.
render(
<MCPServerEdit
mcpServer={{
...interactiveOAuthServer,
auth_type: "oauth2",
credentials: null,
has_configured_client: true,
}}
accessToken="access-token"
userID="user-1"
onCancel={vi.fn()}
onSuccess={vi.fn()}
availableAccessGroups={[]}
/>,
);
await selectAntOption("Authentication", "True Passthrough (no LiteLLM auth)");
await act(async () => {
fireEvent.change(screen.getByPlaceholderText("https://your-mcp-server.com"), {
target: { value: "https://different.example.com/mcp" },
});
});
expect(screen.queryByText(/registered for the previous upstream/)).not.toBeInTheDocument();
});
it("preserves a stored client_id on OAuth-resume restore even when the saved snapshot is token-only", async () => {
// Post-redirect restore: the sessionStorage snapshot carries only a minted token (no client keys),
// while the loaded server has a stored client_id. The restore must merge the server's declared app

12
ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx
View file

@ -4,6 +4,7 @@ import { InfoCircleOutlined } from "@ant-design/icons";
import { Button, TabGroup, TabList, Tab, TabPanels, TabPanel } from "@tremor/react";
import {
AUTH_TYPE,
credentialAuthClass,
isClientForwardedTokenMode,
getOAuthAuthorizationIdentity,
CLEARED_ON_INVALIDATION,
@ -463,6 +464,10 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
// change while a declared app is present keeps the app but flags that it may not match the new
// upstream (the "keep + warn" behavior). Mirrors the create form; independent of the held-token
// stale check so it fires even without an authorize this session (the stored app is for the old url).
// A stored app counts too: the GET redacts credentials to null, so blank fields (keep-existing) can
// still hide a saved client. has_configured_client is the backend's non-secret "a client exists"
// bit; it only warns while the credential class is unchanged, because a cross-class switch replaces
// the stored app on save (blanks then mean "no app"), so there is nothing kept to mismatch.
if ("credentials" in changedValues) {
setAppMayNotMatchUpstream(false);
} else {
@ -470,7 +475,12 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
(key) => key in changedValues,
);
const hasDeclaredApp = preservedDeclaredAppCredentials(form.getFieldValue("credentials")) !== undefined;
if (upstreamChanged && hasDeclaredApp) {
const formAuthType = form.getFieldValue("auth_type") as string | undefined;
const hasStoredKeptApp =
mcpServer.has_configured_client === true &&
isClientForwardedTokenMode(formAuthType) &&
credentialAuthClass(mcpServer.auth_type) === credentialAuthClass(formAuthType);
if (upstreamChanged && (hasDeclaredApp || hasStoredKeptApp)) {
setAppMayNotMatchUpstream(true);
}
}

6
ui/litellm-dashboard/src/components/mcp_tools/types.tsx
View file

@ -371,6 +371,12 @@ export interface MCPServer {
max_concurrent_requests?: number | null;
/** Redacted to null in server responses; present when constructing a server locally. */
credentials?: Record<string, unknown> | null;
/**
* Response-only: true when the stored (redacted) credentials include an OAuth client_id. Lets the
* edit form know a saved app exists without ever seeing its value, so the "app may not match
* upstream" warning can fire on a URL change even though `credentials` arrives null.
*/
has_configured_client?: boolean | null;
/** Stdio-only fields (present when transport === 'stdio') */
command?: string | null;