Repository navigation
Skills: URI traversal validation skips percent-encoded dot-segments #3685
Copy link
Copy link
Open
Labels
bugSomething isn't workingSomething isn't workingv2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)
Description
Initial Checks
- I confirm that I'm using the newest release of my line (the latest 2.x, or the latest 1.x if I'm still on v1)
- I confirm that I searched for my issue in https://github.com/modelcontextprotocol/python-sdk/issues before opening this issue
Release line
2.x (current stable)
Description
In src/mcp/shared/skills.py (added in #3485 for the SEP-2640 Skills extension), _validate_resource_uri_in_skill() and validate_directory_result() check for directory traversal segments against the raw URI path segments:
# src/mcp/shared/skills.py (line 253)
if any(segment in (".", "..") for segment in resource_parts.path.split("/")):
raise ValueError(f"resource URI {resource_uri!r} contains a traversal segment")Because URI paths are not URL-unquoted prior to validation, percent-encoded traversal segments (e.g. %2e%2e, %2E%2E, or %2e) bypass this check.
Specifically:
- In
_validate_resource_uri_in_skill(), a resource URI such asskill://acme/billing/refunds/%2e%2e/other/secret.txtstarts withroot + "/"(/billing/refunds/), but its segments (["", "billing", "refunds", "%2e%2e", "other", "secret.txt"]) do not literally equal".."or".". As a result, the resource is admitted as a valid member ofskill://acme/billing/refunds/SKILL.md. - In
validate_directory_result(), line 293 checksrelative in (".", ".."). A child URI containing%2e%2eor%2ebypasses this check and is admitted as a valid direct child.
Example Code
from mcp.shared.skills import Skill, SkillResource, ReadDirectoryResult, validate_directory_result
from mcp_types import Resource
root = "skill://git-workflow/SKILL.md"
digest = "sha256:" + "a" * 64
# 1. Percent-encoded traversal bypasses manifest validation:
s = Skill(
uri=root,
frontmatter={"name": "git-workflow", "description": "d"},
resources=[
SkillResource(uri=root, digest=digest, size=10),
SkillResource(uri="skill://git-workflow/%2e%2e/evil.md", digest=digest, size=10)
]
)
# Expected: ValueError / ValidationError for traversal segment
# Observed: Accepted without error
# 2. Percent-encoded dot-segment bypasses directory validation:
res = ReadDirectoryResult(resources=[Resource(uri="skill://pdf/templates/%2e%2e", name="x")])
validate_directory_result("skill://pdf/templates", res)
# Expected: ValueError ("is not a direct child")
# Observed: Accepted without errorProposed Fix
Normalize path segments with urllib.parse.unquote() before splitting and inspecting dot-segments in both _validate_resource_uri_in_skill and validate_directory_result.
I have a minimal regression test and fix ready and would be happy to submit a PR if this looks good.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
bugSomething isn't workingSomething isn't workingv2Affects the v2 line (2.x on main)Affects the v2 line (2.x on main)
Activity
Ben-Home commentedon Oct 12, 2026
Good catch. The validation is checking the wrong string: percent-decoding belongs before any segment comparison, and the check has to run on the canonical form, not the raw URI. Concretely:
Decode, then normalize, then validate: unquote() first, then split and check for '.', '..', and empty segments. One decode is not enough either: %252e%252e decodes to %2e%2e, which decodes again to '..'. Loop the decode until stable (bounded), or reject any residual % in a segment after one decode when the path came from untrusted input.
Run the same check for directory result children, not only resource URIs. Both call sites quoted above compare the raw string, so both need the same fix.
Defense in depth on the serving side: after resolution, verify the real path stays inside the skill root (realpath(join(root, rel)).startswith(realpath(root) + os.sep)) before touching the filesystem. Segment validation catches encoding bypasses; the containment check catches symlinks and platform quirks the parser did not anticipate.
Test vectors worth pinning in the suite: '%2e%2e', '%2E%2E', '%2e', plain '.' and '..', doubled encodings, and mixed '%2e.' sequences.
We ship an MCP server that resolves resource URIs across connected sources (corpusiq.io), so this class sits on our permanent test list. The decode-before-validate ordering is the fix that actually holds.
Varshith-Kali commentedon Oct 13, 2026
I'd like to take this on. Plan: percent-decode the URI path before the traversal analysis in both
_validate_resource_uri_in_skill()andvalidate_directory_result()(a URI resolves after decoding per RFC 3986 §2, so%2e%2eis..to the reader), which also exposes..%2F-style sequences as..segments. Could a maintainer assign this to me? Happy to open the PR once assigned.chauvuusvn commentedon Oct 14, 2026
@Varshith-Kali Thanks for offering to help! Since I had already investigated the root cause, implemented the fix, and verified the regression test suite across percent-encoded variants on my fork, I have just submitted the PR in #3703. Please feel free to review the PR and share any feedback there!