Skip to content

fix: forbid extra arguments in FastMCP tool arg models (#3067)#3068

Open
elbachir-salik wants to merge 5 commits into
modelcontextprotocol:mainfrom
elbachir-salik:fix/argmodel-forbid-extra-3067
Open

fix: forbid extra arguments in FastMCP tool arg models (#3067)#3068
elbachir-salik wants to merge 5 commits into
modelcontextprotocol:mainfrom
elbachir-salik:fix/argmodel-forbid-extra-3067

Conversation

@elbachir-salik

Copy link
Copy Markdown

Closes #3067
ArgModelBase now uses extra="forbid" so unknown or misspelled tool arguments raise a validation error instead of being silently ignored.
Added a regression test confirming unknown args are rejected.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 10 files

Re-trigger cubic

@STiFLeR7

Copy link
Copy Markdown

Reviewed against current `upstream/main`: `ArgModelBase.model_config` is the single Pydantic base every FastMCP-generated tool arg model inherits (`func_metadata.py`), and it's built purely from the function's declared signature params (Context-typed params are already excluded before the model is constructed, confirmed via the existing `test_context.py` snapshot tests) — there's no code path here that legitimately needs to accept an argument name outside the declared schema, and no `**kwargs`-style dynamic-arg support exists in `func_metadata.py` today. So `extra="forbid"` closes a real hole (misspelled/unknown `tools/call` arguments silently vanishing instead of failing) without a regression path for the current codebase.

Also checked that the updated `additionalProperties: false` in the JSON Schema output matches the new `extra="forbid"` semantics one-to-one (Pydantic sets that schema keyword automatically for forbid-mode models), so client-side schema validation and server-side runtime validation stay in sync — no silent divergence between what the tool advertises and what it enforces.

Tests cover both the schema-shape change (existing snapshot tests updated) and the new runtime behavior (`test_unknown_argument_raises_validation_error`, plus the doc-tutorial test that now asserts rejection instead of silent drop). CI is green across the full Python matrix. Looks correct to me.

@elbachir-salik

Copy link
Copy Markdown
Author

Reviewed against current upstream/main: ArgModelBase.model_config is the single Pydantic base every FastMCP-generated tool arg model inherits (func_metadata.py), and it's built purely from the function's declared signature params (Context-typed params are already excluded before the model is constructed, confirmed via the existing test_context.py snapshot tests) — there's no code path here that legitimately needs to accept an argument name outside the declared schema, and no **kwargs-style dynamic-arg support exists in func_metadata.py today. So extra="forbid" closes a real hole (misspelled/unknown tools/call arguments silently vanishing instead of failing) without a regression path for the current codebase.

Also checked that the updated additionalProperties: false in the JSON Schema output matches the new extra="forbid" semantics one-to-one (Pydantic sets that schema keyword automatically for forbid-mode models), so client-side schema validation and server-side runtime validation stay in sync — no silent divergence between what the tool advertises and what it enforces.

Tests cover both the schema-shape change (existing snapshot tests updated) and the new runtime behavior (test_unknown_argument_raises_validation_error, plus the doc-tutorial test that now asserts rejection instead of silent drop). CI is green across the full Python matrix. Looks correct to me.

Thanks for the thorough review! Glad the fix holds up across all the edge cases you checked.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FastMCP tool argument models silently ignore unknown/misspelled arguments (extra=ignore default)

2 participants