diff --git a/src/mcp_runtime/fastmcp_output.py b/src/mcp_runtime/fastmcp_output.py index e1cd1ba..5428a9b 100644 --- a/src/mcp_runtime/fastmcp_output.py +++ b/src/mcp_runtime/fastmcp_output.py @@ -11,6 +11,9 @@ FastMCP validates every returned dict against the model; undeclared keys are dropped, so the annotation is the complete list of keys a client can see. + +Arguments are made to match: :func:`_forbid_undeclared_arguments` makes an +undeclared *input* an error, where before it was accepted and silently dropped. """ from types import UnionType @@ -80,6 +83,35 @@ def _offers_message(schema: dict[str, Any]) -> bool: ) +def _forbid_undeclared_arguments(converted: FastMCPTool) -> None: + """Reject arguments a tool does not declare, rather than dropping them. + + ``to_fastmcp`` upstream publishes the input schema from the LangChain tool's + ``tool_call_schema`` but validates against a *separate* model, built from + that schema's fields alone:: + + arg_model = create_model(..., **field_definitions, __base__=ArgModelBase) + + ``ArgModelBase`` sets no extra policy, so pydantic's default applies: an + argument the tool does not declare is dropped, the tool runs without it, and + nothing anywhere says so. + + That is a silent wrong answer rather than a loud failure: the call succeeds, + the result looks ordinary, and the one thing that would explain it — that an + argument went missing — is the one thing nobody is told. A validation error + naming the parameter is something a model reads and corrects instead. + + Both halves are needed. Forbidding extras on the model is the enforcement; + ``additionalProperties: false`` on the published schema is what tells a + client the rule exists. Enforcing without publishing would refuse calls the + advertised schema allowed, which is its own kind of surprise. + """ + arg_model = converted.fn_metadata.arg_model + arg_model.model_config["extra"] = "forbid" + arg_model.model_rebuild(force=True) + converted.parameters["additionalProperties"] = False + + def to_fastmcp(tool: BaseTool) -> FastMCPTool: """Convert a LangChain tool to FastMCP, deriving its output schema. @@ -87,6 +119,7 @@ def to_fastmcp(tool: BaseTool) -> FastMCPTool: not follow the ToolResult contract — see the module docstring. """ converted = _to_fastmcp(tool) + _forbid_undeclared_arguments(converted) annotation = _return_annotation(tool) if annotation is None or not all( _structured_dict(arm) for arm in _arms(annotation) diff --git a/tests/mcp_runtime/test_fastmcp_output.py b/tests/mcp_runtime/test_fastmcp_output.py index 01e2617..faa15bc 100644 --- a/tests/mcp_runtime/test_fastmcp_output.py +++ b/tests/mcp_runtime/test_fastmcp_output.py @@ -162,3 +162,33 @@ async def mixed(text: str) -> ToolResult | str: with pytest.raises(RuntimeError, match="mixed"): to_fastmcp(mixed) + + +async def test_an_undeclared_argument_is_refused_not_dropped(): + """Upstream builds the validation model from the schema's fields alone, so + ``model_config`` is lost and pydantic's default drops unknown arguments. + + The caller is never told, which is the dangerous part: a parameter the model + meant to send simply is not there, and the tool runs as though it had never + been asked for. + """ + converted = to_fastmcp(probe) + + with pytest.raises(Exception, match="query_typo"): + await converted.run({"query": "hello", "query_typo": "hello"}) + + +async def test_a_declared_argument_still_gets_through(): + converted = to_fastmcp(probe) + + assert await run_structured(converted, {"query": "hello"}) == { + "message": "Found 1 for 'hello'.", + "items": [{"id": "hello"}], + } + + +def test_the_input_schema_says_so_too(): + """Upstream publishes no extra policy at all, so enforcing one on its own + would refuse calls the advertised schema allowed. The rule has to be in the + schema for a client to apply it, or to know why it was refused.""" + assert to_fastmcp(probe).parameters["additionalProperties"] is False