Skip to content

Commit c1a1787

Browse files
committed
refactor: remove post-decode structural checks from UriTemplate.match
UriTemplate.match() no longer rejects decoded values containing characters like /, ?, #, &. It now faithfully returns whatever expand() would have encoded, so match(expand(x)) == x holds for all inputs. The previous check broke round-trip for legitimate values (a&b expanded to a%26b but match rejected it) and was inconsistent with every other MCP SDK. The spec's own canonical example file:///{path} requires multi-segment values; Kotlin and C# already decode without rejection and document handler-side validation as the security contract. Path-safety validation remains in ResourceSecurity (configurable) and safe_join (the gold-standard check). The %2F path-traversal attack vector is still blocked: ..%2Fetc%2Fpasswd decodes to ../etc/passwd, which contains_path_traversal rejects. Tests confirm this end-to-end. This aligns us with Kotlin's documented model: decode once, pass to handler, handler validates.
1 parent 1987340 commit c1a1787

File tree

6 files changed

+69
-104
lines changed

6 files changed

+69
-104
lines changed

docs/migration.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -566,8 +566,9 @@ By default, extracted parameter values are now rejected if they:
566566

567567
- Contain `..` as a path component (e.g., `..`, `../etc`, `a/../../b`)
568568
- Look like an absolute filesystem path (e.g., `/etc/passwd`, `C:\Windows`)
569-
- Decode to contain structural delimiters that their operator forbids
570-
(e.g., `%2F` smuggled into a simple `{name}`)
569+
570+
These checks apply to the decoded value, so they catch traversal
571+
regardless of encoding (`../etc`, `..%2Fetc`, `%2E%2E/etc` all caught).
571572

572573
If your template parameters legitimately contain `..` (e.g., git commit
573574
ranges like `HEAD~3..HEAD`) or absolute paths, exempt them:

docs/server/resources.md

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -140,8 +140,10 @@ Before your handler runs, the SDK rejects any parameter that:
140140

141141
- contains `..` as a path component
142142
- looks like an absolute path (`/etc/passwd`, `C:\Windows`)
143-
- smuggles a delimiter through URL encoding (for example, `%2F` in a
144-
plain `{name}` where `/` isn't allowed)
143+
144+
These checks apply to the decoded value, so they catch traversal
145+
regardless of how it was encoded in the URI (`../etc`, `..%2Fetc`,
146+
`%2E%2E/etc`, `..%5Cetc` all get caught).
145147

146148
A request that trips these checks is treated as a non-match: the SDK
147149
raises `ResourceError("Unknown resource: {uri}")`, which the client
@@ -271,8 +273,8 @@ templates (below) if you have any, then raise for anything else.
271273
### Templates
272274

273275
The template engine `MCPServer` uses lives in `mcp.shared.uri_template`
274-
and works on its own. You get the same parsing, matching, and
275-
structural checks; you wire up the routing and policy yourself.
276+
and works on its own. You get the same parsing and matching; you wire
277+
up the routing and security policy yourself.
276278

277279
#### Matching requests
278280

@@ -306,8 +308,8 @@ server = Server("my-server", on_read_resource=on_read_resource)
306308
```
307309

308310
`UriTemplate.match()` returns the extracted variables or `None`. URL
309-
decoding and the structural checks (rejecting `%2F` in simple `{name}`
310-
and so on) happen inside `match()`, the same as in `MCPServer`.
311+
decoding happens inside `match()`; the decoded values are returned
312+
as-is without path-safety validation.
311313

312314
Values come out as strings. Convert them yourself: `int(vars["id"])`,
313315
`Path(vars["path"])`, whatever your handler needs.

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,10 @@
2525
class ResourceSecurity:
2626
"""Security policy applied to extracted resource template parameters.
2727
28-
These checks run **after** :meth:`~mcp.shared.uri_template.UriTemplate.match`
29-
has already enforced structural integrity (e.g., rejected ``%2F`` in
30-
simple ``{var}``). They catch semantic attacks that structural checks
31-
cannot: ``..`` traversal and absolute-path injection work even with
32-
perfectly-formed URI components.
28+
These checks run after :meth:`~mcp.shared.uri_template.UriTemplate.match`
29+
has extracted and decoded parameter values. They catch path-traversal
30+
and absolute-path injection regardless of how the value was encoded in
31+
the URI (literal, ``%2F``, ``%5C``, ``%2E%2E``).
3332
3433
Example::
3534
@@ -153,10 +152,9 @@ def from_function(
153152
def matches(self, uri: str) -> dict[str, str | list[str]] | None:
154153
"""Check if a URI matches this template and extract parameters.
155154
156-
Delegates to :meth:`UriTemplate.match` for RFC 6570 matching
157-
with structural integrity (``%2F`` smuggling rejected for simple
158-
vars), then applies this template's :class:`ResourceSecurity`
159-
policy (path traversal, absolute paths).
155+
Delegates to :meth:`UriTemplate.match` for RFC 6570 extraction,
156+
then applies this template's :class:`ResourceSecurity` policy
157+
(path traversal, absolute paths).
160158
161159
Returns:
162160
Extracted parameters on success, or ``None`` if the URI

src/mcp/shared/uri_template.py

Lines changed: 12 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -75,20 +75,6 @@ class _OperatorSpec:
7575
"&": r"[^&#]*", # query-cont value
7676
}
7777

78-
# Characters that must not appear in a DECODED value for each operator.
79-
# If %2F smuggles a / into a simple {var}, the decoded value violates
80-
# the template author's declared structure and the match is rejected.
81-
_STRUCTURAL_FORBIDDEN: dict[Operator, frozenset[str]] = {
82-
"": frozenset("/?#&"),
83-
"+": frozenset(),
84-
"#": frozenset(),
85-
".": frozenset("./?#"),
86-
"/": frozenset("/?#"),
87-
";": frozenset(";/?#"),
88-
"?": frozenset("&#"),
89-
"&": frozenset("&#"),
90-
}
91-
9278

9379
class InvalidUriTemplate(ValueError):
9480
"""Raised when a URI template string is malformed or unsupported.
@@ -343,16 +329,15 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di
343329
"""Match a concrete URI against this template and extract variables.
344330
345331
This is the inverse of :meth:`expand`. The URI is matched against
346-
a regex derived from the template; captured values are
347-
percent-decoded and validated for structural integrity.
332+
a regex derived from the template and captured values are
333+
percent-decoded. For any value ``v``, ``match(expand({k: v}))``
334+
returns ``{k: v}``.
348335
349-
**Structural integrity**: decoded values must not contain
350-
characters that are structurally significant for their operator.
351-
A simple ``{name}`` whose value decodes to contain ``/`` is
352-
rejected — if that was intended, the template author should use
353-
``{+name}``. This blocks the ``%2F``-smuggling vector where a
354-
client encodes a path separator to bypass single-segment
355-
semantics.
336+
Matching is structural at the URI level only: a simple ``{name}``
337+
will not match across a literal ``/`` in the URI (the regex stops
338+
there), but a percent-encoded ``%2F`` that decodes to ``/`` is
339+
accepted as part of the value. Path-safety validation belongs at
340+
a higher layer; see :mod:`mcp.shared.path_security`.
356341
357342
Example::
358343
@@ -361,8 +346,6 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di
361346
{'name': 'readme.txt'}
362347
>>> t.match("file://docs/hello%20world.txt")
363348
{'name': 'hello world.txt'}
364-
>>> t.match("file://docs/..%2Fetc%2Fpasswd") is None # / in simple var
365-
True
366349
367350
>>> t = UriTemplate.parse("file://docs/{+path}")
368351
>>> t.match("file://docs/src/main.py")
@@ -381,8 +364,7 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di
381364
Returns:
382365
A mapping from variable names to decoded values (``str`` for
383366
scalar variables, ``list[str]`` for explode variables), or
384-
``None`` if the URI does not match the template, a decoded
385-
value violates structural integrity, or the URI exceeds
367+
``None`` if the URI does not match the template or exceeds
386368
``max_uri_length``.
387369
"""
388370
if len(uri) > max_uri_length:
@@ -395,12 +377,10 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di
395377
# One capture group per variable, emitted in template order.
396378
for var, raw in zip(self._variables, m.groups()):
397379
spec = _OPERATOR_SPECS[var.operator]
398-
forbidden = _STRUCTURAL_FORBIDDEN[var.operator]
399380

400381
if var.explode:
401382
# Explode capture holds the whole run including separators,
402-
# e.g. "/a/b/c" or ";keys=a;keys=b". Split, decode each
403-
# segment, check each.
383+
# e.g. "/a/b/c" or ";keys=a;keys=b". Split and decode each.
404384
if not raw:
405385
result[var.name] = []
406386
continue
@@ -419,18 +399,10 @@ def match(self, uri: str, *, max_uri_length: int = DEFAULT_MAX_URI_LENGTH) -> di
419399
seg = ""
420400
else:
421401
return None
422-
decoded = unquote(seg)
423-
if any(c in decoded for c in forbidden):
424-
return None
425-
segments.append(decoded)
402+
segments.append(unquote(seg))
426403
result[var.name] = segments
427404
else:
428-
decoded = unquote(raw)
429-
# Structural integrity: reject if decoding revealed a
430-
# delimiter the operator doesn't permit.
431-
if any(c in decoded for c in forbidden):
432-
return None
433-
result[var.name] = decoded
405+
result[var.name] = unquote(raw)
434406

435407
return result
436408

tests/server/mcpserver/resources/test_resource_template.py

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,9 @@ def test_matches_rfc6570_reserved_expansion():
2626
assert t.matches("file://docs/src/main.py") == {"path": "src/main.py"}
2727

2828

29-
def test_matches_rejects_encoded_slash_in_simple_var():
30-
# Path traversal via encoded slash: %2F smuggled into a simple {var}
29+
def test_matches_rejects_encoded_slash_traversal():
30+
# %2F decodes to / in UriTemplate.match(), giving "../../etc/passwd".
31+
# ResourceSecurity's traversal check then rejects the '..' components.
3132
t = _make("file://docs/{name}")
3233
assert t.matches("file://docs/..%2F..%2Fetc%2Fpasswd") is None
3334

@@ -73,29 +74,35 @@ def test_matches_explode_checks_each_segment():
7374
assert t.matches("api/a/../c") is None
7475

7576

76-
def test_matches_encoded_backslash_caught_by_traversal_layer():
77-
# %5C decodes to '\\'. Backslash is not a URI delimiter, so it passes
78-
# structural integrity (layer 1). The traversal check (layer 2)
79-
# normalizes '\\' to '/' and catches the '..' components.
77+
def test_matches_encoded_backslash_caught_by_traversal_check():
78+
# %5C decodes to '\\'. The traversal check normalizes '\\' to '/'
79+
# and catches the '..' components.
8080
t = _make("file://docs/{name}")
8181
assert t.matches("file://docs/..%5C..%5Csecret") is None
8282

8383

84-
def test_matches_encoded_dots_caught_by_traversal_layer():
85-
# %2E%2E decodes to '..'. Contains no structural delimiter, so passes
86-
# layer 1. Layer 2's traversal check catches the '..' component.
84+
def test_matches_encoded_dots_caught_by_traversal_check():
85+
# %2E%2E decodes to '..' which the traversal check rejects.
8786
t = _make("file://docs/{name}")
8887
assert t.matches("file://docs/%2E%2E") is None
8988

9089

9190
def test_matches_mixed_encoded_and_literal_slash():
92-
# One encoded slash + one literal: literal '/' prevents the regex
93-
# match at layer 0 (simple var stops at '/'), so this never reaches
94-
# decoding. Different failure mode than pure-encoded traversal.
91+
# The literal '/' stops the simple-var regex, so the URI doesn't
92+
# match the template at all.
9593
t = _make("file://docs/{name}")
9694
assert t.matches("file://docs/..%2F../etc") is None
9795

9896

97+
def test_matches_encoded_slash_without_traversal_allowed():
98+
# %2F decoding to '/' is fine when there's no traversal involved.
99+
# UriTemplate accepts it; ResourceSecurity only blocks '..' and
100+
# absolute paths. Handlers that need single-segment should use
101+
# safe_join or validate explicitly.
102+
t = _make("file://docs/{name}")
103+
assert t.matches("file://docs/sub%2Ffile.txt") == {"name": "sub/file.txt"}
104+
105+
99106
def test_matches_escapes_template_literals():
100107
# Regression: old impl treated . as regex wildcard
101108
t = _make("data://v1.0/{id}")

tests/shared/test_uri_template.py

Lines changed: 20 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -410,54 +410,38 @@ def test_match_escapes_template_literals():
410410

411411

412412
@pytest.mark.parametrize(
413-
("template", "uri"),
413+
("template", "uri", "expected"),
414414
[
415-
# %2F in simple var — encoded-slash path traversal
416-
("file://docs/{name}", "file://docs/..%2F..%2Fetc%2Fpasswd"),
417-
("file://docs/{name}", "file://docs/..%2f..%2fetc%2fpasswd"),
418-
# %3F (?) in simple var
419-
("{var}", "a%3Fb"),
420-
# %2E (.) in label var — would break label structure
421-
("file{.ext}", "file.a%2Eb"),
422-
# %2F in path-segment var
423-
("api{/v}", "api/a%2Fb"),
424-
# %26 (&) in query var — would break query structure
425-
("search{?q}", "search?q=a%26b"),
415+
# Percent-encoded delimiters round-trip through match/expand.
416+
# Path-safety validation belongs to ResourceSecurity, not here.
417+
("file://docs/{name}", "file://docs/a%2Fb", {"name": "a/b"}),
418+
("{var}", "a%3Fb", {"var": "a?b"}),
419+
("{var}", "a%23b", {"var": "a#b"}),
420+
("{var}", "a%26b", {"var": "a&b"}),
421+
("file{.ext}", "file.a%2Eb", {"ext": "a.b"}),
422+
("api{/v}", "api/a%2Fb", {"v": "a/b"}),
423+
("search{?q}", "search?q=a%26b", {"q": "a&b"}),
424+
("{;filter}", ";filter=a%3Bb", {"filter": "a;b"}),
426425
],
427426
)
428-
def test_match_structural_integrity_rejects_smuggled_delimiters(template: str, uri: str):
429-
assert UriTemplate.parse(template).match(uri) is None
427+
def test_match_encoded_delimiters_roundtrip(template: str, uri: str, expected: dict[str, str]):
428+
assert UriTemplate.parse(template).match(uri) == expected
430429

431430

432-
def test_match_structural_integrity_allows_slash_in_reserved():
433-
# {+var} explicitly permits / — structural check must not block it
431+
def test_match_reserved_expansion_handles_slash():
432+
# {+var} allows literal / (not just encoded)
434433
t = UriTemplate.parse("{+path}")
435434
assert t.match("a%2Fb") == {"path": "a/b"}
436435
assert t.match("a/b") == {"path": "a/b"}
437436

438437

439438
def test_match_double_encoding_decoded_once():
440439
# %252F is %2F encoded again. Single decode gives "%2F" (a literal
441-
# percent sign, a '2', and an 'F'), which contains no '/' and should
442-
# be accepted. Guards against over-decoding.
440+
# percent sign, a '2', and an 'F'). Guards against over-decoding.
443441
t = UriTemplate.parse("file://docs/{name}")
444442
assert t.match("file://docs/..%252Fetc") == {"name": "..%2Fetc"}
445443

446444

447-
def test_match_multi_param_one_poisoned_rejects_whole():
448-
# One bad param in a multi-param template rejects the entire match
449-
t = UriTemplate.parse("file://{org}/{repo}")
450-
assert t.match("file://acme/..%2Fsecret") is None
451-
# But the same template with clean params matches fine
452-
assert t.match("file://acme/project") == {"org": "acme", "repo": "project"}
453-
454-
455-
def test_match_bare_encoded_delimiter_rejected():
456-
# A value that decodes to only the forbidden delimiter
457-
t = UriTemplate.parse("file://docs/{name}")
458-
assert t.match("file://docs/%2F") is None
459-
460-
461445
def test_match_rejects_oversized_uri():
462446
t = UriTemplate.parse("{var}")
463447
assert t.match("x" * 100, max_uri_length=50) is None
@@ -478,10 +462,11 @@ def test_match_default_uri_length_limit():
478462
assert t.match("x" * (DEFAULT_MAX_URI_LENGTH + 1)) is None
479463

480464

481-
def test_match_structural_integrity_per_explode_segment():
465+
def test_match_explode_encoded_separator_in_segment():
466+
# An encoded separator inside a segment decodes as part of the value,
467+
# not as a split point. The split happens at literal separators only.
482468
t = UriTemplate.parse("/files{/path*}")
483-
# Each segment checked independently
484-
assert t.match("/files/a%2Fb/c") is None
469+
assert t.match("/files/a%2Fb/c") == {"path": ["a/b", "c"]}
485470

486471

487472
@pytest.mark.parametrize(

0 commit comments

Comments
 (0)