Skip to content

Skills: URI traversal validation skips percent-encoded dot-segments #3685

Open

Description

@chauvuusvn

Initial Checks

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:

  1. In _validate_resource_uri_in_skill(), a resource URI such as skill://acme/billing/refunds/%2e%2e/other/secret.txt starts with root + "/" (/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 of skill://acme/billing/refunds/SKILL.md.
  2. In validate_directory_result(), line 293 checks relative in (".", ".."). A child URI containing %2e%2e or %2e bypasses 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 error

Proposed 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.

Activity

  1. added
    bugSomething isn't working
    v2Affects the v2 line (2.x on main)
    on Oct 12, 2026
  2. Ben-Home commented on Oct 12, 2026

    @Ben-Home

    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:

    1. 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.

    2. 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.

    3. 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.

  3. Varshith-Kali commented on Oct 13, 2026

    @Varshith-Kali

    I'd like to take this on. Plan: percent-decode the URI path before the traversal analysis in both _validate_resource_uri_in_skill() and validate_directory_result() (a URI resolves after decoding per RFC 3986 §2, so %2e%2e is .. 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.

  4. chauvuusvn commented on Oct 14, 2026

    @chauvuusvn
    Author

    @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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingv2Affects the v2 line (2.x on main)

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions