fix(mcp): bind inherited preview credentials to the saved origin

This commit is contained in:
Joshua Valluru 2026-09-09 18:01:58 -07:00
parent 245369764e
commit 3523f3731c
5 changed files with 91 additions and 10 deletions

View file

@ -1255,7 +1255,18 @@ if MCP_AVAILABLE:
MCPRequestHandler,
)
request: Final = _inherit_credentials_from_existing_server(new_mcp_server_request)
saved_server: Final = (
global_mcp_server_manager.get_mcp_server_by_id(new_mcp_server_request.server_id)
if new_mcp_server_request.server_id
else None
)
saved_origin: Final = _redact_mcp_resource_url(saved_server.url) if saved_server else None
may_inherit: Final = new_mcp_server_request.auth_type not in _STAGED_AUTH_VALUE_AUTH_TYPES or (
saved_origin is not None and saved_origin == _redact_mcp_resource_url(new_mcp_server_request.url)
)
request: Final = (
_inherit_credentials_from_existing_server(new_mcp_server_request) if may_inherit else new_mcp_server_request
)
mcp_auth_header: Final = (
request.credentials.get("auth_value")
if request.auth_type in _STAGED_AUTH_VALUE_AUTH_TYPES and isinstance(request.credentials, dict)

View file

@ -128,7 +128,7 @@ class TestExecuteWithMcpClient:
payload: Final = NewMCPServerRequest(
server_id=saved.server_id,
server_name="edited",
url="https://edited.example/mcp",
url="https://stored.example/corrected-mcp",
transport=MCPTransport.sse,
auth_type=auth_type,
credentials={"auth_value": auth_value} if auth_value is not None else None,
@ -146,13 +146,48 @@ class TestExecuteWithMcpClient:
oauth2_headers=staged.oauth2_headers,
)
assert result == {
"url": "https://edited.example/mcp",
"url": "https://stored.example/corrected-mcp",
"transport": MCPTransport.sse,
"headers": {"X-Preview": "edited", **expected_auth},
}
assert manager.get_mcp_server_by_id(saved.server_id) is saved
assert saved.url == "https://stored.example/mcp"
@pytest.mark.parametrize(
"url", ("https://other.example/mcp", "http://stored.example/mcp", "https://stored.example:8443/mcp")
)
@pytest.mark.parametrize("explicit_credential", (None, "preview:explicit"))
def test_static_preview_does_not_inherit_credentials_across_origins(
self, monkeypatch: pytest.MonkeyPatch, url: str, explicit_credential: str | None
) -> None:
from starlette.datastructures import Headers
from litellm.proxy._experimental.mcp_server.mcp_server_manager import MCPServerManager
from litellm.proxy.management_endpoints import mcp_management_endpoints
saved: Final = MCPServer(
server_id="saved-preview-server",
name="saved",
url="https://stored.example/mcp",
transport=MCPTransport.http,
auth_type=MCPAuth.basic,
authentication_token="preview:stored",
)
manager: Final = MCPServerManager()
manager.registry = {saved.server_id: saved}
monkeypatch.setattr(rest_endpoints, "global_mcp_server_manager", manager)
monkeypatch.setattr(mcp_management_endpoints, "global_mcp_server_manager", manager)
payload: Final = NewMCPServerRequest(
server_id=saved.server_id,
url=url,
transport=MCPTransport.http,
auth_type=MCPAuth.basic,
credentials={"auth_value": explicit_credential} if explicit_credential else None,
)
staged: Final = rest_endpoints._stage_server_test(payload, Headers())
assert staged.mcp_auth_header == explicit_credential
assert staged.request.credentials == payload.credentials
@pytest.mark.asyncio
async def test_redacts_stack_trace(self, monkeypatch):
async def fake_create_client(*args, **kwargs):

View file

@ -14,10 +14,10 @@ describe("getEditToolPreview", () => {
});
it("previews URL changes with the existing server credential left for server-side inheritance", () => {
expect(getEditToolPreview({ ...saved, url: "https://correct.example/mcp" }, saved)).toEqual({
expect(getEditToolPreview({ ...saved, url: "https://example.com/corrected-mcp" }, saved)).toEqual({
kind: "preview",
config: {
url: "https://correct.example/mcp",
url: "https://example.com/corrected-mcp",
transport: "http",
auth_type: "basic",
static_headers: { "X-Tenant": "original" },
@ -26,6 +26,24 @@ describe("getEditToolPreview", () => {
});
});
it.each(["https://other.example/mcp", "http://example.com/mcp", "https://example.com:8443/mcp"])(
"requires explicit credentials for a changed origin: %s",
(url) => {
expect(getEditToolPreview({ ...saved, url, static_headers: [] }, saved)).toEqual({
kind: "incomplete",
message: expect.stringContaining("origin changed"),
});
const explicit = { ...saved, url, static_headers: [], credentials: { auth_value: "new:secret" } };
expect(getEditToolPreview(explicit, saved).kind).toBe("preview");
},
);
it("does not automatically send saved static headers to a new origin", () => {
expect(getEditToolPreview({ ...saved, url: "https://other.example/mcp", auth_type: "none" }, saved).kind).toBe(
"incomplete",
);
});
it("uses edited static headers and only the static auth value", () => {
expect(
getEditToolPreview(

View file

@ -21,7 +21,7 @@ const connectionConfig = (values: Readonly<Record<string, unknown>>) => {
type EditToolPreview =
| { readonly kind: "saved" }
| { readonly kind: "incomplete" }
| { readonly kind: "incomplete"; readonly message?: string }
| { readonly kind: "preview"; readonly config: ReturnType<typeof connectionConfig> };
export const getEditToolPreview = (
@ -49,5 +49,19 @@ export const getEditToolPreview = (
if (!validUrl || missingNewCredential || incompleteHeaders) {
return { kind: "incomplete" };
}
const savedConfig = connectionConfig(initialValues);
const changedOrigin =
!URL.canParse(savedConfig.url) || new URL(config.url).origin !== new URL(savedConfig.url).origin;
const reusesHeader = Object.entries(config.static_headers).some(
([key, value]) => savedConfig.static_headers[key] === value,
);
const needsSavedCredential = AUTH_TYPES_REQUIRING_AUTH_VALUE.includes(config.auth_type) && !config.credentials;
if (changedOrigin && (needsSavedCredential || reusesHeader)) {
return {
kind: "incomplete",
message:
"The server origin changed. Enter credentials and replace or remove saved static headers to preview tools.",
};
}
return { kind: "preview", config };
};

View file

@ -454,19 +454,22 @@ const MCPServerEdit: React.FC<MCPServerEditProps> = ({
const toolPreviewKey = JSON.stringify(toolPreview);
useEffect(() => {
let active = true;
const controller = new AbortController();
setTools([]);
setToolsError(null);
setIsLoadingTools(false);
if (!accessToken || !mcpServer.server_id) return;
if (toolPreview.kind === "incomplete") {
setToolsError("Complete the URL, authentication, and header settings to load tools.");
setToolsError(toolPreview.message ?? "Complete the URL, authentication, and header settings to load tools.");
return;
}
setIsLoadingTools(true);
const timer = setTimeout(() => fetchTools(() => active), toolPreview.kind === "preview" ? 500 : 0);
const timer = setTimeout(
() => fetchTools(() => !controller.signal.aborted),
toolPreview.kind === "preview" ? 500 : 0,
);
return () => {
active = false;
controller.abort();
clearTimeout(timer);
};
// eslint-disable-next-line react-hooks/exhaustive-deps