You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Python: Add models for the MCP server SDKs (mcp, fastmcp) - #22703
Implements the proposal in #22702. The observation there about py/xxe, py/xml-bomb and py/nosql-injection taking only RemoteFlowSource has since been fixed by #22822.
MCP servers expose tools to LLM agents. Every parameter of a tool, resource or prompt handler is
chosen by the client, that is by a model that prompt injection can steer, or by anyone who can
reach the server over HTTP. This PR models that input for the two Python SDKs as Models-as-Data
next to the existing Stdlib.model.yml sources: the handler parameters as sources of the opt-in ai threat model (#22810), the transport headers and the bearer token as sources of the remote
threat model.
Covered:
mcp 1.x FastMCP and 2.x MCPServer: tool(), resource(), prompt(), add_tool(fn).
The low-level Server: the 1.x decorators call_tool(), read_resource(), get_prompt(), and
the 2.x constructor handlers on_call_tool, on_read_resource, on_get_prompt, where params.arguments / params.uri is the source.
fastmcp 2.x to 4.x: tool and prompt bare, called or as a plain call, resource(), add_tool, add_prompt, Tool.from_function, fastmcp.tools.tool, the transport headers via get_http_headers() and get_http_request().headers.
Both SDKs: the bearer token of the Authorization header, in verify_token of TokenVerifier
(and AuthProvider) subclasses and from get_access_token().token.
typeModel rows for the re-exported import paths of the classes.
Tests: library-tests/frameworks/mcp and library-tests/frameworks/fastmcp use MaDTest
(mad-source__ai, mad-source__remote); every row has a handler with an inline expectation, and classes with the
same method name but no SDK base class have none.
Evidence: with these rows (as a model pack) CodeQL 2.27.1 finds 13 of 28 real, publicly disclosed
MCP server vulnerabilities in mcp-vulnbench
instead of 1; on the half of the cases that was held out while writing the rows, 9 of 16 instead
of 0, at 0.27 to 1.09 alarms per KLOC. Known limits of this form (handlers behind project
decorators, Pydantic-typed parameters, headers read through the Context object) are documented
there and in the issue. The measurement used the rows as remote, which is on by default; with ai the same sources apply once that threat model is enabled.
Handler parameters are produced by a language model, so they are sources of the opt-in ai threat model added in github#22810. HTTP headers and bearer tokens come from the HTTP client and stay sources of the remote threat model.
Following the suggestion in #22749, the handler parameters are now sources of the opt-in ai threat model (#22810); the HTTP headers and bearer tokens stay remote, since they come from the HTTP client. Rebased on main. With #22822 merged, the note in the description about py/xxe, py/xml-bomb and py/nosql-injection is settled: these queries now see sources from data extensions.
I have merged main into the branch to pick up the ai source kind in the model validation from #22749; without it, library-tests/frameworks/data fails on these rows with Invalid kind "ai" in source model. The diff of this PR is unchanged. Locally (CodeQL 2.27.1), frameworks/data, frameworks/mcp and frameworks/fastmcp pass.
The reason will be displayed to describe this comment to others. Learn more.
Parameter[any] includes ctx: Context, although the SDK injects this rather than taking it from client arguments. I think the only fix is to use QL for these models.
The reason will be displayed to describe this comment to others. Learn more.
Done in f215487: the handlers of FastMCP, MCPServer and fastmcp are now modeled in Mcp.qll, which leaves out a parameter annotated with Context (also as Context[...], Context | None, Optional[...], Union[...], Annotated[...], or a string annotation in a module that imports Context) and the receiver of non-static methods; the tests have a negative case for each. The low-level server keeps its rows, since its handlers take (name, arguments) or (ctx, params) and no Context. #22750 also adds a Mcp.qll, for handlers behind a project decorator; I used the same name so that both end up in one module, and I'm happy to do that merge on whichever side lands second.
The reason will be displayed to describe this comment to others. Learn more.
Update: following the Copilot review, 3c696d2 no longer leaves out the receiver (a handler decorated in a class body is registered unbound, so the client supplies self too), and it only looks at type positions of an annotation (not the metadata of Annotated or the values of Literal). frameworks/fastmcp, frameworks/mcp and frameworks/data pass locally.
The reason will be displayed to describe this comment to others. Learn more.
Parameter[any] includes ctx: Context, although the SDK injects this rather than taking it from client arguments. I think the only fix is to use QL for these models.
`Parameter[any]` also matched the `Context` parameter that the SDKs inject,
so the handlers of `FastMCP`, `MCPServer` and `fastmcp` are now modeled in
`Mcp.qll`, which leaves it out (also as `Context | None`, `Optional[...]`,
`Annotated[...]` or a string annotation) as well as the receiver of methods.
This also covers `Prompt.from_function`, `FunctionPrompt.from_function`, the
`Tool` classes in `fastmcp.tools.tool` and the standalone decorators in
`fastmcp.tools.function_tool` and `fastmcp.prompts`. The low-level server
keeps its data extensions and now also models `params.name`.
The reason will be displayed to describe this comment to others. Learn more.
🟡 Changes recommended
MCP parameter classification incorrectly excludes client-controlled inputs and includes aliased SDK-injected context, risking both missed security findings and false positives.
Adds Python CodeQL models for the mcp and fastmcp server SDKs so existing security queries can track client-controlled handler input, headers, and bearer tokens.
Changes:
Models high-level handler parameters in QL under the opt-in ai threat model, excluding injected context parameters.
Adds data-extension sources for low-level handlers, transport headers, and authentication tokens.
Registers the framework model and adds regression tests and a change note.
Only type positions of an annotation refer to the injected `Context`
class: the metadata of `Annotated` and the values of `Literal` do not.
String annotations also match the names under which a module imports
the class, such as `Ctx` in `from fastmcp import Context as Ctx`.
A handler decorated in a class body is registered unbound, so the
client supplies its first parameter too; it is no longer left out.
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Unresolved MCP modeling gaps in annotations, supported imports, dependency injection, and unbound callbacks incorrectly include server-injected parameters as ai sources while omitting client-controlled sources.
Handlers using from mcp.server import FastMCP are not modeled because this supported 1.x re-export is missing from serverClass(). All tool, resource and prompt parameters registered through that import therefore lose their ai sources. Include mcpServer.getMember("FastMCP") and add a regression handler using this import.
A handler using from fastmcp.server import Context is not recognized as receiving injected context. This is a supported re-export, so its SDK-provided ctx incorrectly becomes an ai source and can produce false positives. Add this path to contextClass() and cover the import in the fixture.
For import fastmcp followed by a handler parameter ctx: "fastmcp.Context", there need not be any non-string reference to the Context class in this module. Consequently this exists has no matching ref, contextName returns nothing, and the SDK-injected ctx is incorrectly reported as an ai source. Resolve qualified string annotations through imported module bindings as well as direct class imports, and cover this module-import form in the tests.
Recognize aliased Annotated and Literal annotations
With from typing import Annotated as A or Literal as L, annotations such as A[str, "Context"] and L["Context", "Global"] enter the generic-subscript branch instead of the type-position checks. Their metadata or value strings then cause client-controlled parameters to be excluded from ai sources. Resolve these wrappers through their typing/typing_extensions bindings and add aliased-form regression cases.
Recognize Context subclasses as injected parameters
MCP 2.x's find_context_parameter also injects parameters annotated with subclasses of Context. For class CustomContext(Context): ... and a handler parameter ctx: CustomContext, this check only follows values from the imported base class, so ctx incorrectly becomes an ai source. Include transitive subclasses when recognizing context annotations and add a subclass-annotated regression case.
Fully quoted annotations such as command: "Annotated[str, 'Context']" and scope: "Literal['Context', 'Global']" still match this regex when the module imports Context. These parameters are client-controlled, but matching their metadata or literal values removes their ai sources. Apply type-position checks within quoted annotations too, and add quoted counterparts to the existing regression cases.
FastMCP excludes parameters with Depends(...) defaults from the client-visible schema and resolves them on the server (https://gofastmcp.com/servers/dependency-injection). Here, command: str = Depends(get_fixed_command) still becomes an ai source even when the dependency returns a constant, which can produce a false command-injection finding. Exclude recognized dependency defaults for FastMCP handlers and add a negative regression case alongside the ordinary default-argument case.
Use declared parameters for unbound class callbacks
Low-level handlers declared in a class body are registered unbound, but these paths use API-graph indexing that drops the first declared parameter. This loses the URI in @server.read_resource() on async def read(uri) and misses params for an unbound constructor callback declared as handle(ctx, params). Select the actual declared parameters for unbound callbacks, including the first one for decorators, and add class-body cases for both registration styles.
The reason will be displayed to describe this comment to others. Learn more.
The metadata/Literal fix still fails with aliased typing imports and quoted annotations. With Context imported as SDKContext, these client-controlled parameters are incorrectly excluded:
from typing import Annotated as A, Literal as L
command: A[str, "SDKContext"]
scope: L["SDKContext", "Global"]
command: "Annotated[str, 'SDKContext']"
scope: "Literal['SDKContext', 'Global']"
The wrapper check only recognizes the spelling Annotated/Literal, while the string fallback searches metadata and literal values too. Could we recognize the imported typing symbols, preserve type-position checks for quoted annotations, and add regression tests?
The reason will be displayed to describe this comment to others. Learn more.
Returning "Context" unconditionally is unsafe when the SDK class was imported under another name. For example:
from fastmcp import Context as SDKContext, FastMCP
from pydantic import BaseModel
app = FastMCP("test")
class Context(BaseModel):
command: str
@app.tool()
def run(data: "Context"):
return data.command
Here data is client-controlled, but the model excludes it because its annotation contains "Context". Could we require the quoted name to resolve to the SDK class rather than merely matching a name associated with the module? Please add this as a regression test.
This branch has not been deployed
No deployments
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
owen-mc
left a comment
Implements the proposal in #22702. The observation there about
py/xxe,py/xml-bombandpy/nosql-injectiontaking onlyRemoteFlowSourcehas since been fixed by #22822.MCP servers expose tools to LLM agents. Every parameter of a tool, resource or prompt handler is
chosen by the client, that is by a model that prompt injection can steer, or by anyone who can
reach the server over HTTP. This PR models that input for the two Python SDKs as Models-as-Data
next to the existing
Stdlib.model.ymlsources: the handler parameters as sources of the opt-inaithreat model (#22810), the transport headers and the bearer token as sources of theremotethreat model.
Covered:
mcp1.xFastMCPand 2.xMCPServer:tool(),resource(),prompt(),add_tool(fn).Server: the 1.x decoratorscall_tool(),read_resource(),get_prompt(), andthe 2.x constructor handlers
on_call_tool,on_read_resource,on_get_prompt, whereparams.arguments/params.uriis the source.fastmcp2.x to 4.x:toolandpromptbare, called or as a plain call,resource(),add_tool,add_prompt,Tool.from_function,fastmcp.tools.tool, the transport headers viaget_http_headers()andget_http_request().headers.Authorizationheader, inverify_tokenofTokenVerifier(and
AuthProvider) subclasses and fromget_access_token().token.typeModelrows for the re-exported import paths of the classes.Tests:
library-tests/frameworks/mcpandlibrary-tests/frameworks/fastmcpuseMaDTest(
mad-source__ai,mad-source__remote); every row has a handler with an inline expectation, and classes with thesame method name but no SDK base class have none.
Evidence: with these rows (as a model pack) CodeQL 2.27.1 finds 13 of 28 real, publicly disclosed
MCP server vulnerabilities in mcp-vulnbench
instead of 1; on the half of the cases that was held out while writing the rows, 9 of 16 instead
of 0, at 0.27 to 1.09 alarms per KLOC. Known limits of this form (handlers behind project
decorators, Pydantic-typed parameters, headers read through the
Contextobject) are documentedthere and in the issue. The measurement used the rows as
remote, which is on by default; withaithe same sources apply once that threat model is enabled.