[v1.x] fix: reject trailing newline in tool-name validation

[maxisbey]

Backport of #3076 to v1.x.

TOOL_NAME_REGEX is $-anchored and checked with re.match; in Python, $ also matches just before a single trailing newline, so a tool name like "name\n" validated cleanly with no SEP-986 warning. Switch the check to re.fullmatch, mirroring the fix merged to main.

Credit to @Otis0408 for finding and fixing the original issue (#3084, #3076).

Note: v1.x has no uri_template.py (that module is v2-only), so only the tool-name half of #3076 applies here.

_{AI Disclaimer}

[cubic-dev-ai]

No issues found across 2 files

_{Re-trigger cubic}

[claude]

LGTM — minimal, correct backport of the re.match → re.fullmatch fix with regression tests; the one inline comment is a pre-existing issue in an untouched file and doesn't block this PR.

Extended reasoning...

Overview

This PR backports #3076 to v1.x: a one-line change in src/mcp/shared/tool_name_validation.py switching TOOL_NAME_REGEX.match(name) to TOOL_NAME_REGEX.fullmatch(name), plus two parametrized regression test cases. It fixes the classic Python pitfall where $ with re.match accepts a single trailing newline, so "name\n" previously validated cleanly with no SEP-986 warning. I verified the semantics: with fullmatch, a match ending before a trailing \n no longer spans the full string, so such names are now correctly rejected. The "a"*127 + "\n" test case is well chosen — at length 128 it passes the length check and exercises the regex path directly.

Security risks

None introduced. The change strictly tightens validation (rejects strings previously accepted), and this validation only produces warnings/rejection flags for tool registration — it is not an auth or injection boundary. No new attack surface.

Level of scrutiny

Low. This is a mechanical, well-understood one-line fix mirroring a change already merged to main, in a self-contained validation utility with dedicated test coverage. The note in the PR description that only the tool-name half of #3076 applies to v1.x is accurate for the files this PR targets; the inline comment notes a separate pre-existing instance of the same bug class in ResourceTemplate.matches() (src/mcp/server/fastmcp/resources/templates.py:89), which is worth a follow-up but is not touched by — and should not block — this PR.

Other factors

The new tests directly regression-test the fixed behavior, and the existing test suite for this module is comprehensive. The change is identical in shape to the fix on main, so there is no design decision requiring human judgment.

[claude]

🟣 Pre-existing issue, not introduced by this PR: the same $-with-re.match trailing-newline bug fixed here for tool names also exists on v1.x in ResourceTemplate.matches() (src/mcp/server/fastmcp/resources/templates.py:89), where 'resource://user/123\n' matches template 'resource://user/{id}' with the newline captured into the extracted parameter. The wire path is protected by pydantic AnyUrl normalization, but FastMCP.read_resource/Context.read_resource accept raw str, so consider a follow-up switching it to re.fullmatch — note this also means the PR description's claim that the URI half of #3076 doesn't apply to v1.x isn't quite complete.

Extended reasoning...

The bug

This PR fixes the classic Python regex pitfall — $ matches not only at end-of-string but also just before a single trailing \n, so re.match(r'^...$', s) accepts strings with a trailing newline — for TOOL_NAME_REGEX in tool_name_validation.py. The identical pattern remains on v1.x in ResourceTemplate.matches():

# src/mcp/server/fastmcp/resources/templates.py:85-92
def matches(self, uri: str) -> dict[str, Any] | None:
    """Check if URI matches template and extract parameters."""
    pattern = self.uri_template.replace("{", "(?P<").replace("}", ">[^/]+)")
    match = re.match(f"^{pattern}$", uri)
    if match:
        return match.groupdict()
    return None

Step-by-step proof

For template resource://user/{id} and input 'resource://user/123\n':

1. The template is converted to the pattern ^resource://user/(?P<id>[^/]+)$.
2. [^/]+ matches any non-slash character — including \n — so it greedily consumes 123\n.
3. $ (without re.MULTILINE) matches at end-of-string or just before a trailing newline at end-of-string; here it matches at the very end after [^/]+ consumed the newline.
4. Result: match.groupdict() returns {'id': '123\n'} — the URI is accepted and the trailing newline is smuggled into the extracted parameter, which is then passed as an argument to the resource function.

(Even if [^/]+ were non-greedy, step 3 would let $ match before the newline — either way the input is accepted when it should be rejected.)

Why the impact is limited

On the wire-protocol path, ReadResourceRequestParams.uri is a pydantic AnyUrl, and pydantic-core normalization strips a raw trailing newline ('resource://user/123\n' validates to 'resource://user/123'), so a remote client sending a compliant resources/read request cannot trigger this. However, the server-side entry points FastMCP.read_resource, Context.read_resource, and ResourceManager.get_resource all accept AnyUrl | str and pass a plain str through unnormalized, so in-process/programmatic callers still hit the bug.

Relation to this PR

This PR does not touch templates.py, so this is squarely pre-existing and should not block merge. It is worth noting, though, because the PR description states that the URI half of #3076 doesn't apply to v1.x (since uri_template.py is v2-only) — but this separate fastmcp instance of the same bug class does exist on v1.x, so the backport leaves one instance unfixed.

How to fix

Mirror this PR: use re.fullmatch(pattern, uri) instead of re.match(f'^{pattern}$', uri). Alternatively, anchor with \A...\Z — which src/mcp/server/experimental/task_scope.py:31-34 already does, with a comment explicitly warning about this exact $/trailing-newline hazard, so the fix direction is already established in this codebase.