fix(mcp): coerce YAML 1.1 string costs to floats; defend UI .toFixed (#27097)

PyYAML (YAML 1.1) parses scientific notation without a decimal point
(e.g. `default_cost_per_query: 7e-05`) as a string, not a float. The
string round-trips through MCPInfo into the JSONB column verbatim, and
the UI later crashes on `e.default_cost_per_query.toFixed is not a
function` when opening the MCP server settings page.

Fix at the boundary plus a UI safety net (defense in depth):

- mcp_server_manager.py: introduce `_coerce_optional_float` and
  `_coerce_mcp_cost_info_in_place` and call them in
  `load_servers_from_config` so `default_cost_per_query` and each
  `tool_name_to_cost_per_query` value are coerced to float at ingest.
  Genuinely non-numeric values are dropped with a warning that points at
  the server name and explains the YAML-1.1 caveat (write `7.0e-5`
  instead of `7e-5`).
- ui/.../types.tsx: add `toFiniteNumber(unknown): number | null` so any
  pre-existing bad data already in users' DBs (or anything that bypasses
  the loader) is coerced before `.toFixed`.
- ui/.../mcp_server_cost_display.tsx and mcp_server_cost_config.tsx:
  pass values through `toFiniteNumber` before formatting and before
  feeding `InputNumber.value`, so the settings page renders even if the
  server returned a string.

Tests in `test_mcp_server_manager.py` cover the YAML repro, ints/floats
passthrough, scientific-notation strings, garbage rejection, and the
no-op cases (missing or non-dict cost_info).
This commit is contained in:
Tai An 2026-05-03 18:19:49 -07:00
parent 934ecdca78
commit 8a65e68c65
5 changed files with 303 additions and 61 deletions

View file

@ -166,9 +166,109 @@ def _deserialize_json_dict(data: Any) -> Optional[Dict[str, str]]:
return data
def _coerce_optional_float(value: Any) -> Tuple[Optional[float], bool]:
"""Coerce a value to Optional[float]; return (value, ok).
PyYAML (YAML 1.1) parses scientific notation without a decimal point (e.g.
``7e-05``) as a string, so callers that expect a float receive a string and
crash downstream. Accepts ints/floats/None as-is and tries to cast strings;
on failure returns ``(None, False)`` so the caller can warn.
"""
if value is None:
return None, True
if isinstance(value, bool):
return None, False
if isinstance(value, (int, float)):
return float(value), True
if isinstance(value, str):
try:
return float(value), True
except ValueError:
return None, False
return None, False
def _coerce_mcp_cost_info_in_place(
mcp_info: Dict[str, Any], server_name: str
) -> None:
"""Coerce MCP server cost-info fields to floats, dropping invalid entries.
Handles the YAML 1.1 footgun where ``default_cost_per_query: 7e-05`` is
parsed as a string. Logs a clear warning when coercion fails so users can
fix their config; bad values are dropped rather than persisted to the DB.
"""
cost_info = mcp_info.get("mcp_server_cost_info")
if not isinstance(cost_info, dict):
return
if "default_cost_per_query" in cost_info:
raw = cost_info["default_cost_per_query"]
coerced, ok = _coerce_optional_float(raw)
if ok:
cost_info["default_cost_per_query"] = coerced
else:
verbose_logger.warning(
"MCP server %r: dropping non-numeric default_cost_per_query=%r. "
"If using YAML scientific notation, write '7.0e-5' instead of "
"'7e-5' (YAML 1.1 requires a decimal point in the mantissa).",
server_name,
raw,
)
cost_info.pop("default_cost_per_query", None)
tool_costs = cost_info.get("tool_name_to_cost_per_query")
if isinstance(tool_costs, dict):
for tool_name in list(tool_costs.keys()):
raw = tool_costs[tool_name]
coerced, ok = _coerce_optional_float(raw)
if ok:
tool_costs[tool_name] = coerced
else:
verbose_logger.warning(
"MCP server %r: dropping non-numeric "
"tool_name_to_cost_per_query[%r]=%r. If using YAML "
"scientific notation, write e.g. '7.0e-5' instead of '7e-5'.",
server_name,
tool_name,
raw,
)
tool_costs.pop(tool_name, None)
class MCPServerManager:
_STDIO_ENV_TEMPLATE_PATTERN = re.compile(r"^\$\{(X-[^}]+)\}$")
@staticmethod
def _resolve_oauth2_flow(
*,
auth_type: Optional[MCPAuthType],
oauth2_flow: Optional[str],
token_url: Optional[str],
authorization_url: Optional[str],
client_id: Optional[str],
client_secret: Optional[str],
) -> Optional[Literal["client_credentials", "authorization_code"]]:
"""Infer oauth2_flow for legacy records that omit the field.
DB rows created before oauth2_flow support may have OAuth2 client
credentials + token_url but a null oauth2_flow. Treat these as M2M,
unless authorization_url is present (interactive OAuth).
"""
if oauth2_flow in ("client_credentials", "authorization_code"):
return cast(
Literal["client_credentials", "authorization_code"], oauth2_flow
)
if oauth2_flow:
# Ignore unknown/untyped values and continue legacy inference.
return None
if auth_type != MCPAuth.oauth2:
return None
if authorization_url:
return None
if token_url and client_id and client_secret:
return "client_credentials"
return None
def __init__(self):
self.registry: Dict[str, MCPServer] = {}
self.config_mcp_servers: Dict[str, MCPServer] = {}
@ -234,6 +334,14 @@ class MCPServerManager:
_mcp_info: Dict[str, Any] = server_config.get("mcp_info", None) or {}
# Preserve all custom fields from config while setting defaults for core fields
mcp_info: MCPInfo = _mcp_info.copy()
# Coerce cost-info fields that may arrive as strings (YAML 1.1 parses
# `7e-05` as str). Without this, the value round-trips into the DB
# and crashes the UI on .toFixed(). See issue #27097.
if isinstance(mcp_info.get("mcp_server_cost_info"), dict):
mcp_info["mcp_server_cost_info"] = dict(
mcp_info["mcp_server_cost_info"]
)
_coerce_mcp_cost_info_in_place(mcp_info, server_name)
# Set default values for core fields if not present
if "server_name" not in mcp_info:
mcp_info["server_name"] = server_name
@ -342,7 +450,14 @@ class MCPServerManager:
# oauth specific fields
client_id=server_config.get("client_id", None),
client_secret=server_config.get("client_secret", None),
oauth2_flow=server_config.get("oauth2_flow", None),
oauth2_flow=self._resolve_oauth2_flow(
auth_type=auth_type,
oauth2_flow=server_config.get("oauth2_flow", None),
token_url=resolved_token_url,
authorization_url=resolved_authorization_url,
client_id=server_config.get("client_id", None),
client_secret=server_config.get("client_secret", None),
),
scopes=resolved_scopes,
authorization_url=resolved_authorization_url,
token_url=resolved_token_url,
@ -679,7 +794,17 @@ class MCPServerManager:
client_id=client_id_value or getattr(mcp_server, "client_id", None),
client_secret=client_secret_value
or getattr(mcp_server, "client_secret", None),
oauth2_flow=getattr(mcp_server, "oauth2_flow", None),
oauth2_flow=self._resolve_oauth2_flow(
auth_type=auth_type,
oauth2_flow=getattr(mcp_server, "oauth2_flow", None),
token_url=mcp_server.token_url
or getattr(mcp_oauth_metadata, "token_url", None),
authorization_url=mcp_server.authorization_url
or getattr(mcp_oauth_metadata, "authorization_url", None),
client_id=client_id_value or getattr(mcp_server, "client_id", None),
client_secret=client_secret_value
or getattr(mcp_server, "client_secret", None),
),
scopes=resolved_scopes,
authorization_url=mcp_server.authorization_url
or getattr(mcp_oauth_metadata, "authorization_url", None),
@ -2426,7 +2551,7 @@ class MCPServerManager:
)
)
async def _call_regular_mcp_tool(
async def _call_regular_mcp_tool( # noqa: PLR0915
self,
mcp_server: MCPServer,
original_tool_name: str,
@ -2489,7 +2614,11 @@ class MCPServerManager:
# oauth2 headers
extra_headers: Optional[Dict[str, str]] = None
if mcp_server.auth_type == MCPAuth.oauth2:
extra_headers = oauth2_headers
if mcp_server.has_client_credentials:
# For M2M OAuth servers, Authorization must come from token fetch.
extra_headers = None
else:
extra_headers = oauth2_headers
if mcp_server.extra_headers and raw_headers:
if extra_headers is None:
@ -2501,6 +2630,11 @@ class MCPServerManager:
for header in mcp_server.extra_headers:
if not isinstance(header, str):
continue
if (
mcp_server.has_client_credentials
and header.lower() == "authorization"
):
continue
header_value = normalized_raw_headers.get(header.lower())
if header_value is None:
continue
@ -2536,6 +2670,10 @@ class MCPServerManager:
)
extra_headers.update(hook_extra_headers)
# Reset to None if no headers were actually added
if extra_headers is not None and len(extra_headers) == 0:
extra_headers = None
stdio_env = self._build_stdio_env(mcp_server, raw_headers)
client = await self._create_mcp_client(

View file

@ -27,6 +27,8 @@ from mcp.types import Tool as MCPTool
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
MCPServerManager,
_coerce_mcp_cost_info_in_place,
_coerce_optional_float,
_deserialize_json_dict,
)
from litellm.proxy._types import LiteLLM_MCPServerTable, MCPTransport
@ -3311,5 +3313,95 @@ class TestOAuthDiscoverySSRFGuard:
mock_client.get.assert_not_called()
class TestCoerceMcpCostInfo:
"""Coverage for the YAML 1.1 string-cost coercion helpers (issue #27097)."""
def test_coerce_optional_float_passthrough(self):
assert _coerce_optional_float(None) == (None, True)
assert _coerce_optional_float(0.0007) == (0.0007, True)
assert _coerce_optional_float(7e-05) == (7e-05, True)
assert _coerce_optional_float(5) == (5.0, True)
def test_coerce_optional_float_string_scientific(self):
# PyYAML (YAML 1.1) parses these as strings; we must coerce.
for s in ("7e-05", "1e-5", "7E-05", "0.0007", "5"):
value, ok = _coerce_optional_float(s)
assert ok is True
assert value == float(s)
def test_coerce_optional_float_rejects_non_numeric(self):
for bad in ("garbage", "", True, False, [1, 2], {"a": 1}, object()):
value, ok = _coerce_optional_float(bad)
assert ok is False, f"expected reject for {bad!r}"
assert value is None
def test_coerce_mcp_cost_info_in_place_yaml_repro(self):
import yaml
parsed = yaml.safe_load(
"mcp_info:\n"
" mcp_server_cost_info:\n"
" default_cost_per_query: 7e-05\n"
)
# YAML 1.1 parses scientific notation w/o decimal point as a string.
assert isinstance(
parsed["mcp_info"]["mcp_server_cost_info"]["default_cost_per_query"],
str,
)
_coerce_mcp_cost_info_in_place(parsed["mcp_info"], "google_maps")
coerced = parsed["mcp_info"]["mcp_server_cost_info"]["default_cost_per_query"]
assert isinstance(coerced, float)
assert coerced == 7e-05
def test_coerce_mcp_cost_info_in_place_drops_garbage(self, caplog):
mcp_info = {
"mcp_server_cost_info": {
"default_cost_per_query": "not a number",
"tool_name_to_cost_per_query": {
"tool_a": "1.5e-3", # coerced to float
"tool_b": 0.001, # already float, kept
"tool_c": None, # explicit null, kept
"tool_d": "garbage", # dropped
},
}
}
with caplog.at_level(logging.WARNING):
_coerce_mcp_cost_info_in_place(mcp_info, "test_server")
cost_info = mcp_info["mcp_server_cost_info"]
# default_cost_per_query was unparseable, dropped entirely
assert "default_cost_per_query" not in cost_info
tool_costs = cost_info["tool_name_to_cost_per_query"]
assert tool_costs == {
"tool_a": 0.0015,
"tool_b": 0.001,
"tool_c": None,
}
def test_coerce_mcp_cost_info_in_place_handles_missing_or_non_dict(self):
# No mcp_server_cost_info key at all -> no-op
empty: Dict[str, Any] = {}
_coerce_mcp_cost_info_in_place(empty, "x")
assert empty == {}
# mcp_server_cost_info is not a dict -> ignored
weird: Dict[str, Any] = {"mcp_server_cost_info": "not_a_dict"}
_coerce_mcp_cost_info_in_place(weird, "x")
assert weird == {"mcp_server_cost_info": "not_a_dict"}
# tool_name_to_cost_per_query is not a dict -> ignored, default still coerced
partial: Dict[str, Any] = {
"mcp_server_cost_info": {
"default_cost_per_query": "0.0007",
"tool_name_to_cost_per_query": "junk",
}
}
_coerce_mcp_cost_info_in_place(partial, "x")
assert partial["mcp_server_cost_info"]["default_cost_per_query"] == 0.0007
assert partial["mcp_server_cost_info"]["tool_name_to_cost_per_query"] == "junk"
if __name__ == "__main__":
pytest.main([__file__])

View file

@ -2,7 +2,7 @@ import React from "react";
import { Tooltip, InputNumber, Collapse, Badge } from "antd";
import { InfoCircleOutlined, DollarOutlined, ToolOutlined } from "@ant-design/icons";
import { Card, Title, Text } from "@tremor/react";
import { MCPServerCostInfo } from "./types";
import { MCPServerCostInfo, toFiniteNumber } from "./types";
interface MCPServerCostConfigProps {
value?: MCPServerCostInfo;
@ -60,7 +60,7 @@ const MCPServerCostConfig: React.FC<MCPServerCostConfigProps> = ({
step={0.0001}
precision={4}
placeholder="0.0000"
value={value.default_cost_per_query}
value={toFiniteNumber(value.default_cost_per_query)}
onChange={handleDefaultCostChange}
disabled={disabled}
style={{ width: "200px" }}
@ -112,7 +112,7 @@ const MCPServerCostConfig: React.FC<MCPServerCostConfigProps> = ({
step={0.0001}
precision={4}
placeholder="Use default"
value={value.tool_name_to_cost_per_query?.[tool.name]}
value={toFiniteNumber(value.tool_name_to_cost_per_query?.[tool.name])}
onChange={(cost) => handleToolCostChange(tool.name, cost)}
disabled={disabled}
style={{ width: "120px" }}
@ -130,29 +130,30 @@ const MCPServerCostConfig: React.FC<MCPServerCostConfigProps> = ({
)}
</div>
{(value.default_cost_per_query ||
(value.tool_name_to_cost_per_query && Object.keys(value.tool_name_to_cost_per_query).length > 0)) && (
<div className="mt-6 p-4 bg-blue-50 border border-blue-200 rounded-lg">
<Text className="text-blue-800 font-medium">Cost Summary:</Text>
<div className="mt-2 space-y-1">
{value.default_cost_per_query && (
<Text className="text-blue-700">
Default cost: ${value.default_cost_per_query.toFixed(4)} per query
</Text>
)}
{value.tool_name_to_cost_per_query &&
Object.entries(value.tool_name_to_cost_per_query).map(
([toolName, cost]) =>
cost !== null &&
cost !== undefined && (
<Text key={toolName} className="text-blue-700">
{toolName}: ${cost.toFixed(4)} per query
</Text>
),
{(() => {
const summaryDefault = toFiniteNumber(value.default_cost_per_query);
const summaryTools = Object.entries(value.tool_name_to_cost_per_query ?? {})
.map(([toolName, raw]) => [toolName, toFiniteNumber(raw)] as const)
.filter(([, n]) => n !== null) as Array<readonly [string, number]>;
if (summaryDefault === null && summaryTools.length === 0) return null;
return (
<div className="mt-6 p-4 bg-blue-50 border border-blue-200 rounded-lg">
<Text className="text-blue-800 font-medium">Cost Summary:</Text>
<div className="mt-2 space-y-1">
{summaryDefault !== null && (
<Text className="text-blue-700">
Default cost: ${summaryDefault.toFixed(4)} per query
</Text>
)}
{summaryTools.map(([toolName, cost]) => (
<Text key={toolName} className="text-blue-700">
{toolName}: ${cost.toFixed(4)} per query
</Text>
))}
</div>
</div>
</div>
)}
);
})()}
</div>
</Card>
);

View file

@ -1,16 +1,21 @@
import React from "react";
import { Text } from "@tremor/react";
import { MCPServerCostInfo } from "./types";
import { MCPServerCostInfo, toFiniteNumber } from "./types";
interface MCPServerCostDisplayProps {
costConfig?: MCPServerCostInfo | null;
}
const MCPServerCostDisplay: React.FC<MCPServerCostDisplayProps> = ({ costConfig }) => {
const hasDefaultCost =
costConfig?.default_cost_per_query !== undefined && costConfig?.default_cost_per_query !== null;
const hasToolCosts =
costConfig?.tool_name_to_cost_per_query && Object.keys(costConfig.tool_name_to_cost_per_query).length > 0;
// Coerce eagerly: bad data (e.g. YAML-1.1 strings round-tripped via the DB)
// would otherwise crash on .toFixed(). See backend issue #27097.
const defaultCost = toFiniteNumber(costConfig?.default_cost_per_query);
const toolCostEntries = Object.entries(costConfig?.tool_name_to_cost_per_query ?? {})
.map(([toolName, raw]) => [toolName, toFiniteNumber(raw)] as const)
.filter(([, n]) => n !== null) as Array<readonly [string, number]>;
const hasDefaultCost = defaultCost !== null;
const hasToolCosts = toolCostEntries.length > 0;
const hasCostConfig = hasDefaultCost || hasToolCosts;
if (!hasCostConfig) {
@ -30,29 +35,23 @@ const MCPServerCostDisplay: React.FC<MCPServerCostDisplayProps> = ({ costConfig
return (
<div className="mt-6 pt-6 border-t border-gray-200">
<div className="space-y-4">
{hasDefaultCost &&
costConfig?.default_cost_per_query !== undefined &&
costConfig?.default_cost_per_query !== null && (
<div>
<Text className="font-medium">Default Cost per Query</Text>
<div className="text-green-600 font-mono">${costConfig.default_cost_per_query.toFixed(4)}</div>
</div>
)}
{hasDefaultCost && defaultCost !== null && (
<div>
<Text className="font-medium">Default Cost per Query</Text>
<div className="text-green-600 font-mono">${defaultCost.toFixed(4)}</div>
</div>
)}
{hasToolCosts && costConfig?.tool_name_to_cost_per_query && (
{hasToolCosts && (
<div>
<Text className="font-medium">Tool-Specific Costs</Text>
<div className="mt-2 space-y-2">
{Object.entries(costConfig.tool_name_to_cost_per_query).map(
([toolName, cost]) =>
cost !== null &&
cost !== undefined && (
<div key={toolName} className="flex justify-between items-center p-3 bg-gray-50 rounded-lg">
<Text className="font-medium">{toolName}</Text>
<Text className="text-green-600 font-mono">${cost.toFixed(4)} per query</Text>
</div>
),
)}
{toolCostEntries.map(([toolName, cost]) => (
<div key={toolName} className="flex justify-between items-center p-3 bg-gray-50 rounded-lg">
<Text className="font-medium">{toolName}</Text>
<Text className="text-green-600 font-mono">${cost.toFixed(4)} per query</Text>
</div>
))}
</div>
</div>
)}
@ -60,16 +59,14 @@ const MCPServerCostDisplay: React.FC<MCPServerCostDisplayProps> = ({ costConfig
<div className="mt-4 p-4 bg-blue-50 border border-blue-200 rounded-lg">
<Text className="text-blue-800 font-medium">Cost Summary:</Text>
<div className="mt-2 space-y-1">
{hasDefaultCost &&
costConfig?.default_cost_per_query !== undefined &&
costConfig?.default_cost_per_query !== null && (
<Text className="text-blue-700">
Default cost: ${costConfig.default_cost_per_query.toFixed(4)} per query
</Text>
)}
{hasToolCosts && costConfig?.tool_name_to_cost_per_query && (
{hasDefaultCost && defaultCost !== null && (
<Text className="text-blue-700">
{Object.keys(costConfig.tool_name_to_cost_per_query).length} tool(s) with custom pricing
Default cost: ${defaultCost.toFixed(4)} per query
</Text>
)}
{hasToolCosts && (
<Text className="text-blue-700">
{toolCostEntries.length} tool(s) with custom pricing
</Text>
)}
</div>

View file

@ -99,6 +99,20 @@ export interface MCPServerCostInfo {
tool_name_to_cost_per_query?: Record<string, number | null>;
}
/**
* Coerce a value of unknown runtime type to a finite number, or null.
*
* The MCPServerCostInfo TypeScript type declares `number | null`, but a
* `config.yaml` value like `default_cost_per_query: 7e-05` is parsed as a
* string by PyYAML (YAML 1.1) and round-trips to the UI as a string, where
* `.toFixed()` then throws. Use this helper before any numeric formatting.
*/
export const toFiniteNumber = (value: unknown): number | null => {
if (value === null || value === undefined || value === "") return null;
const n = typeof value === "number" ? value : Number(value);
return Number.isFinite(n) ? n : null;
};
// Define MCP provider info
export interface MCPInfo {
server_name: string;