From e04b721502bde6873d1b26e3ae4d7039001594e1 Mon Sep 17 00:00:00 2001 From: otiscuilei Date: Wed, 8 Jul 2026 19:11:56 +0800 Subject: [PATCH 1/2] fix: reject trailing newline in tool-name validation TOOL_NAME_REGEX was end-anchored with $, which in Python's default mode also matches just before a single trailing newline. So validate_tool_name("x\n") returned is_valid=True with no warning, and a 127-char name plus "\n" (len 128) slipped past both the length and character checks. Anchor with \Z so a trailing newline is treated as the disallowed character it is. --- src/mcp/shared/tool_name_validation.py | 6 ++++-- tests/shared/test_tool_name_validation.py | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/mcp/shared/tool_name_validation.py b/src/mcp/shared/tool_name_validation.py index f35efa5a61..9614399108 100644 --- a/src/mcp/shared/tool_name_validation.py +++ b/src/mcp/shared/tool_name_validation.py @@ -17,8 +17,10 @@ logger = logging.getLogger(__name__) -# Regular expression for valid tool names according to SEP-986 specification -TOOL_NAME_REGEX = re.compile(r"^[A-Za-z0-9._-]{1,128}$") +# Regular expression for valid tool names according to SEP-986 specification. +# End-anchored with \Z rather than $: in Python's default mode $ also matches +# just before a single trailing newline, which would let "name\n" validate. +TOOL_NAME_REGEX = re.compile(r"^[A-Za-z0-9._-]{1,128}\Z") # SEP reference URL for warning messages SEP_986_URL = "https://modelcontextprotocol.io/specification/2025-11-25/server/tools#tool-names" diff --git a/tests/shared/test_tool_name_validation.py b/tests/shared/test_tool_name_validation.py index 97b3dffcd3..f03e1a52a8 100644 --- a/tests/shared/test_tool_name_validation.py +++ b/tests/shared/test_tool_name_validation.py @@ -80,6 +80,22 @@ def test_validate_tool_name_rejects_invalid_characters(tool_name: str, expected_ assert any("invalid characters" in w and expected_char in w for w in result.warnings) +@pytest.mark.parametrize( + "tool_name", + ["valid_name\n", "a" * 127 + "\n"], + ids=["trailing_newline", "max_length_plus_newline"], +) +def test_validate_tool_name_rejects_trailing_newline(tool_name: str) -> None: + """A trailing newline is not an allowed character and must be rejected. + + Regression test: Python's ``$`` (unlike ``\\Z``) also matches just before a + single trailing newline, so a name like ``"valid_name\\n"`` slipped through. + """ + result = validate_tool_name(tool_name) + assert result.is_valid is False + assert any("invalid characters" in w for w in result.warnings) + + def test_validate_tool_name_rejects_multiple_invalid_chars() -> None: """Names with multiple invalid chars should list all of them.""" result = validate_tool_name("user name@domain,com") From 23102e3e330a84a98602170c12f62e16e4e50a5e Mon Sep 17 00:00:00 2001 From: Max Isbey <224885523+maxisbey@users.noreply.github.com> Date: Fri, 10 Jul 2026 12:50:10 +0000 Subject: [PATCH 2/2] fix: validate whole strings with re.fullmatch With re.match, a $-anchored pattern also matches just before a single trailing newline, so tool-name validation accepted "name\n". Switch the tool-name and URI-template varname checks to re.fullmatch, which puts the whole-string requirement at the call site, and fold the regression cases into the existing invalid-character parametrizations. --- src/mcp/shared/tool_name_validation.py | 8 +++----- src/mcp/shared/uri_template.py | 2 +- tests/shared/test_tool_name_validation.py | 21 +++++---------------- tests/shared/test_uri_template.py | 2 ++ 4 files changed, 11 insertions(+), 22 deletions(-) diff --git a/src/mcp/shared/tool_name_validation.py b/src/mcp/shared/tool_name_validation.py index 9614399108..96c34f7826 100644 --- a/src/mcp/shared/tool_name_validation.py +++ b/src/mcp/shared/tool_name_validation.py @@ -17,10 +17,8 @@ logger = logging.getLogger(__name__) -# Regular expression for valid tool names according to SEP-986 specification. -# End-anchored with \Z rather than $: in Python's default mode $ also matches -# just before a single trailing newline, which would let "name\n" validate. -TOOL_NAME_REGEX = re.compile(r"^[A-Za-z0-9._-]{1,128}\Z") +# Regular expression for valid tool names according to SEP-986 specification +TOOL_NAME_REGEX = re.compile(r"^[A-Za-z0-9._-]{1,128}$") # SEP reference URL for warning messages SEP_986_URL = "https://modelcontextprotocol.io/specification/2025-11-25/server/tools#tool-names" @@ -79,7 +77,7 @@ def validate_tool_name(name: str) -> ToolNameValidationResult: warnings.append("Tool name starts or ends with a dot, which may cause parsing issues in some contexts") # Check for invalid characters - if not TOOL_NAME_REGEX.match(name): + if not TOOL_NAME_REGEX.fullmatch(name): # Find all invalid characters (unique, preserving order) invalid_chars: list[str] = [] seen: set[str] = set() diff --git a/src/mcp/shared/uri_template.py b/src/mcp/shared/uri_template.py index dc57bfa757..20d6fa9c2e 100644 --- a/src/mcp/shared/uri_template.py +++ b/src/mcp/shared/uri_template.py @@ -813,7 +813,7 @@ def _parse_expression(template: str, body: str, pos: int) -> _Expression: explode = spec.endswith("*") name = spec[:-1] if explode else spec - if not _VARNAME_RE.match(name): + if not _VARNAME_RE.fullmatch(name): raise InvalidUriTemplate( f"Invalid variable name {name!r} at position {pos}", template=template, diff --git a/tests/shared/test_tool_name_validation.py b/tests/shared/test_tool_name_validation.py index f03e1a52a8..cbf27b8792 100644 --- a/tests/shared/test_tool_name_validation.py +++ b/tests/shared/test_tool_name_validation.py @@ -65,12 +65,17 @@ def test_validate_tool_name_rejects_name_exceeding_max_length() -> None: ("get,user,profile", "','"), ("user/profile/update", "'/'"), ("user@domain.com", "'@'"), + # a single trailing newline slipped past `$` with re.match + ("valid_name\n", "'\\n'"), + ("a" * 127 + "\n", "'\\n'"), ], ids=[ "with_spaces", "with_commas", "with_slashes", "with_at_symbol", + "with_trailing_newline", + "max_length_with_trailing_newline", ], ) def test_validate_tool_name_rejects_invalid_characters(tool_name: str, expected_char: str) -> None: @@ -80,22 +85,6 @@ def test_validate_tool_name_rejects_invalid_characters(tool_name: str, expected_ assert any("invalid characters" in w and expected_char in w for w in result.warnings) -@pytest.mark.parametrize( - "tool_name", - ["valid_name\n", "a" * 127 + "\n"], - ids=["trailing_newline", "max_length_plus_newline"], -) -def test_validate_tool_name_rejects_trailing_newline(tool_name: str) -> None: - """A trailing newline is not an allowed character and must be rejected. - - Regression test: Python's ``$`` (unlike ``\\Z``) also matches just before a - single trailing newline, so a name like ``"valid_name\\n"`` slipped through. - """ - result = validate_tool_name(tool_name) - assert result.is_valid is False - assert any("invalid characters" in w for w in result.warnings) - - def test_validate_tool_name_rejects_multiple_invalid_chars() -> None: """Names with multiple invalid chars should list all of them.""" result = validate_tool_name("user name@domain,com") diff --git a/tests/shared/test_uri_template.py b/tests/shared/test_uri_template.py index 48f6c66237..cddbb82b0b 100644 --- a/tests/shared/test_uri_template.py +++ b/tests/shared/test_uri_template.py @@ -144,6 +144,8 @@ def test_parse_rejects_operator_without_variable(): # RFC ยง2.3: dots only between varchars, not consecutive or trailing "foo..bar", "foo.", + # a single trailing newline slipped past `$` with re.match + "foo\n", ], ) def test_parse_rejects_invalid_varname(name: str):