fix: raise ResourceSecurityError instead of falling through on rejection

ResourceTemplate.matches() previously returned None for both "URI
doesn't match this template" and "URI matches but fails security
validation". ResourceManager.get_resource iterates templates and uses
the first non-None result, so a strict template's security rejection
would silently fall through to a later, possibly permissive, template.
Registration order became security-critical without documentation.

matches() now raises ResourceSecurityError on security failure,
halting template iteration at the first rejection. The error carries
the template string and the offending parameter name.

ResourceSecurity.validate() now returns the name of the first failing
parameter (or None if all pass) rather than a bool, so the error can
identify which parameter was rejected.

Author: maxisbey

src/mcp/server/mcpserver/resources/__init__.py

@@ -3,6 +3,7 @@
 from .templates import (
     DEFAULT_RESOURCE_SECURITY,
     ResourceSecurity,
+    ResourceSecurityError,
     ResourceTemplate,
 )
 from .types import (
@@ -25,5 +26,6 @@
     "ResourceTemplate",
     "ResourceManager",
     "ResourceSecurity",
+    "ResourceSecurityError",
     "DEFAULT_RESOURCE_SECURITY",
 ]

src/mcp/server/mcpserver/resources/templates.py

@@ -54,34 +54,49 @@ def git_diff(range: str) -> str: ...
     exempt_params: Set[str] = field(default_factory=frozenset[str])
     """Parameter names to skip all checks for."""
 
-    def validate(self, params: Mapping[str, str | list[str]]) -> bool:
+    def validate(self, params: Mapping[str, str | list[str]]) -> str | None:
         """Check all parameter values against the configured policy.
 
         Args:
             params: Extracted template parameters. List values (from
                 explode variables) are checked element-wise.
 
         Returns:
-            ``True`` if all values pass; ``False`` on first violation.
+            The name of the first parameter that fails, or ``None`` if
+            all values pass.
         """
         for name, value in params.items():
             if name in self.exempt_params:
                 continue
             values = value if isinstance(value, list) else [value]
             for v in values:
                 if self.reject_null_bytes and "\0" in v:
-                    return False
+                    return name
                 if self.reject_path_traversal and contains_path_traversal(v):
-                    return False
+                    return name
                 if self.reject_absolute_paths and is_absolute_path(v):
-                    return False
-        return True
+                    return name
+        return None
 
 
 DEFAULT_RESOURCE_SECURITY = ResourceSecurity()
 """Secure-by-default policy: traversal and absolute paths rejected."""
 
 
+class ResourceSecurityError(ValueError):
+    """Raised when an extracted parameter fails :class:`ResourceSecurity` checks.
+
+    Distinct from a simple ``None`` non-match so that template
+    iteration can stop at the first security rejection rather than
+    falling through to a later, possibly more permissive, template.
+    """
+
+    def __init__(self, template: str, param: str) -> None:
+        super().__init__(f"Parameter {param!r} of template {template!r} failed security validation")
+        self.template = template
+        self.param = param
+
+
 class ResourceTemplate(BaseModel):
     """A template for dynamically creating resources."""
 
@@ -165,13 +180,21 @@ def matches(self, uri: str) -> dict[str, str | list[str]] | None:
 
         Returns:
             Extracted parameters on success, or ``None`` if the URI
-            doesn't match or a parameter fails security validation.
+            doesn't match the template.
+
+        Raises:
+            ResourceSecurityError: If the URI matches but an extracted
+                parameter fails security validation. Raising (rather
+                than returning ``None``) prevents the resource manager
+                from silently falling through to a later, possibly more
+                permissive, template.
         """
         params = self.parsed_template.match(uri)
         if params is None:
             return None
-        if not self.security.validate(params):
-            return None
+        failed = self.security.validate(params)
+        if failed is not None:
+            raise ResourceSecurityError(self.uri_template, failed)
         return params
 
     async def create_resource(

tests/server/mcpserver/resources/test_resource_template.py

@@ -9,6 +9,7 @@
 from mcp.server.mcpserver.resources.templates import (
     DEFAULT_RESOURCE_SECURITY,
     ResourceSecurity,
+    ResourceSecurityError,
 )
 from mcp.types import Annotations
 
@@ -30,23 +31,27 @@ def test_matches_rejects_encoded_slash_traversal():
     # %2F decodes to / in UriTemplate.match(), giving "../../etc/passwd".
     # ResourceSecurity's traversal check then rejects the '..' components.
     t = _make("file://docs/{name}")
-    assert t.matches("file://docs/..%2F..%2Fetc%2Fpasswd") is None
+    with pytest.raises(ResourceSecurityError, match="'name'"):
+        t.matches("file://docs/..%2F..%2Fetc%2Fpasswd")
 
 
 def test_matches_rejects_path_traversal_by_default():
     t = _make("file://docs/{name}")
-    assert t.matches("file://docs/..") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs/..")
 
 
 def test_matches_rejects_path_traversal_in_reserved_var():
     # Even {+path} gets the traversal check — it's semantic, not structural
     t = _make("file://docs/{+path}")
-    assert t.matches("file://docs/../../etc/passwd") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs/../../etc/passwd")
 
 
 def test_matches_rejects_absolute_path():
     t = _make("file://docs/{+path}")
-    assert t.matches("file://docs//etc/passwd") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs//etc/passwd")
 
 
 def test_matches_allows_dotdot_as_substring():
@@ -71,9 +76,11 @@ def test_matches_rejects_null_byte_by_default():
     # %00 decodes to \x00 which defeats string comparisons
     # ("..\x00" != "..") and can truncate in C extensions.
     t = _make("file://docs/{name}")
-    assert t.matches("file://docs/key%00.txt") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs/key%00.txt")
     # Null byte also defeats the traversal check's component comparison
-    assert t.matches("file://docs/..%00%2Fsecret") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs/..%00%2Fsecret")
 
 
 def test_matches_null_byte_check_can_be_disabled():
@@ -82,24 +89,47 @@ def test_matches_null_byte_check_can_be_disabled():
     assert t.matches("file://docs/key%00.txt") == {"name": "key\x00.txt"}
 
 
+def test_security_rejection_does_not_fall_through_to_next_template():
+    # A strict template's security rejection must halt iteration, not
+    # fall through to a later permissive template. Previously matches()
+    # returned None for both "no match" and "security failed", making
+    # registration order security-critical.
+    strict = _make("file://docs/{name}")
+    lax = _make(
+        "file://docs/{+path}",
+        security=ResourceSecurity(exempt_params={"path"}),
+    )
+    uri = "file://docs/..%2Fsecrets"
+    # Strict matches structurally then fails security -> raises.
+    with pytest.raises(ResourceSecurityError) as exc:
+        strict.matches(uri)
+    assert exc.value.param == "name"
+    # If this raised, the resource manager never reaches the lax
+    # template. Verify the lax template WOULD have accepted it.
+    assert lax.matches(uri) == {"path": "../secrets"}
+
+
 def test_matches_explode_checks_each_segment():
     t = _make("api{/parts*}")
     assert t.matches("api/a/b/c") == {"parts": ["a", "b", "c"]}
     # Any segment with traversal rejects the whole match
-    assert t.matches("api/a/../c") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("api/a/../c")
 
 
 def test_matches_encoded_backslash_caught_by_traversal_check():
     # %5C decodes to '\\'. The traversal check normalizes '\\' to '/'
     # and catches the '..' components.
     t = _make("file://docs/{name}")
-    assert t.matches("file://docs/..%5C..%5Csecret") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs/..%5C..%5Csecret")
 
 
 def test_matches_encoded_dots_caught_by_traversal_check():
     # %2E%2E decodes to '..' which the traversal check rejects.
     t = _make("file://docs/{name}")
-    assert t.matches("file://docs/%2E%2E") is None
+    with pytest.raises(ResourceSecurityError):
+        t.matches("file://docs/%2E%2E")
 
 
 def test_matches_mixed_encoded_and_literal_slash():