From 94982693ab91501158ae6bb44e5e461908851f5e Mon Sep 17 00:00:00 2001 From: thomas <291887306+tsarlandie-oai@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:29:56 +0000 Subject: [PATCH 1/3] fix(skills): validate percent-encoded skill names --- crates/rmcp/Cargo.toml | 1 + crates/rmcp/src/model/skill.rs | 44 ++++++++++++++++++++++++++++++++-- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index b0e77c72a..0494ecede 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -57,6 +57,7 @@ indexmap = { version = "2", features = ["serde"] } tracing = { version = "0.1" } tokio-util = { version = "0.7" } pin-project-lite = "0.2" +percent-encoding = { version = "2", default-features = false } pastey = { version = "0.2.0", optional = true } # oauth2 support oauth2 = { version = "5.0", optional = true, default-features = false } diff --git a/crates/rmcp/src/model/skill.rs b/crates/rmcp/src/model/skill.rs index edd111e91..4837577d2 100644 --- a/crates/rmcp/src/model/skill.rs +++ b/crates/rmcp/src/model/skill.rs @@ -268,10 +268,13 @@ impl Skill { /// Check the rules the spec puts on a skill entry that can be checked /// without reading any file. pub fn validate(&self) -> Result<(), SkillValidationError> { - let Some(name) = skill_name_from_uri(&self.uri) else { + let Some(encoded_name) = skill_name_from_uri(&self.uri) else { return Err(SkillValidationError::UriNotSkillMd(self.uri.clone())); }; - if name != self.frontmatter.name { + let name = self.name(); + if name.contains(['/', '\\', '?', '#']) + || !percent_encoding::percent_decode_str(encoded_name).eq(name.bytes()) + { return Err(SkillValidationError::NameMismatch { uri: self.uri.clone(), name: self.frontmatter.name.clone(), @@ -739,6 +742,21 @@ mod tests { assert_eq!(skill.validate(), Ok(())); } + #[test] + fn accepts_a_percent_encoded_unicode_name() { + for uri in [ + "skill://caf%C3%A9/SKILL.md", + "skill://team/caf%c3%a9/SKILL.md", + ] { + let skill = Skill::new( + uri, + SkillFrontmatter::new("café", "Coffee recipes"), + SkillResources::Files(vec![SkillResource::from_bytes(uri, b"")]), + ); + assert_eq!(skill.validate(), Ok(()), "{uri}"); + } + } + #[test] fn rejects_a_name_that_differs_from_the_uri() { let mut skill = billing_skill(); @@ -749,6 +767,28 @@ mod tests { )); } + #[test] + fn rejects_invalid_encoded_name_matches() { + for (segment, name) in [ + ("caf%FF", "café"), + ("caf%25C3%25A9", "café"), + ("coffee%2Ftea", "coffee/tea"), + ] { + let skill = Skill::new( + format!("skill://team/{segment}/SKILL.md"), + SkillFrontmatter::new(name, "Drinks"), + SkillResources::Dynamic, + ); + assert!( + matches!( + skill.validate(), + Err(SkillValidationError::NameMismatch { .. }) + ), + "{segment}" + ); + } + } + #[test] fn rejects_a_manifest_without_skill_md() { let mut skill = billing_skill(); From e586115f9b19827f1d7341fe86354aa9a91015c4 Mon Sep 17 00:00:00 2001 From: thomas <291887306+tsarlandie-oai@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:34:28 +0000 Subject: [PATCH 2/3] test(skills): use a spell-checkable Unicode fixture --- crates/rmcp/src/model/skill.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/crates/rmcp/src/model/skill.rs b/crates/rmcp/src/model/skill.rs index 4837577d2..93f9d5e28 100644 --- a/crates/rmcp/src/model/skill.rs +++ b/crates/rmcp/src/model/skill.rs @@ -745,12 +745,12 @@ mod tests { #[test] fn accepts_a_percent_encoded_unicode_name() { for uri in [ - "skill://caf%C3%A9/SKILL.md", - "skill://team/caf%c3%a9/SKILL.md", + "skill://r%C3%A9sum%C3%A9/SKILL.md", + "skill://team/r%c3%a9sum%c3%a9/SKILL.md", ] { let skill = Skill::new( uri, - SkillFrontmatter::new("café", "Coffee recipes"), + SkillFrontmatter::new("résumé", "Résumé templates"), SkillResources::Files(vec![SkillResource::from_bytes(uri, b"")]), ); assert_eq!(skill.validate(), Ok(()), "{uri}"); @@ -770,8 +770,8 @@ mod tests { #[test] fn rejects_invalid_encoded_name_matches() { for (segment, name) in [ - ("caf%FF", "café"), - ("caf%25C3%25A9", "café"), + ("r%FFsum%C3%A9", "résumé"), + ("r%25C3%25A9sum%25C3%25A9", "résumé"), ("coffee%2Ftea", "coffee/tea"), ] { let skill = Skill::new( From 47e65f61170f8171bddf083847b45562c12d7ed5 Mon Sep 17 00:00:00 2001 From: thomas <291887306+tsarlandie-oai@users.noreply.github.com> Date: Tue, 3 Nov 2026 22:44:19 +0000 Subject: [PATCH 3/3] fix(skills): reject duplicate percent-encoded URI aliases --- crates/rmcp/Cargo.toml | 2 +- crates/rmcp/src/model/skill.rs | 43 +++++++++++++++++++++++++++------- 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/crates/rmcp/Cargo.toml b/crates/rmcp/Cargo.toml index 0494ecede..1d04fa889 100644 --- a/crates/rmcp/Cargo.toml +++ b/crates/rmcp/Cargo.toml @@ -57,7 +57,7 @@ indexmap = { version = "2", features = ["serde"] } tracing = { version = "0.1" } tokio-util = { version = "0.7" } pin-project-lite = "0.2" -percent-encoding = { version = "2", default-features = false } +percent-encoding = { version = "2", default-features = false, features = ["alloc"] } pastey = { version = "0.2.0", optional = true } # oauth2 support oauth2 = { version = "5.0", optional = true, default-features = false } diff --git a/crates/rmcp/src/model/skill.rs b/crates/rmcp/src/model/skill.rs index 93f9d5e28..24e8fc3bf 100644 --- a/crates/rmcp/src/model/skill.rs +++ b/crates/rmcp/src/model/skill.rs @@ -5,9 +5,9 @@ //! `resources/read` serves the file bytes. The request and result wiring //! lives in the parent module next to the other methods. -#[cfg(feature = "schemars")] use std::borrow::Cow; +use percent_encoding::{CONTROLS, percent_decode_str, utf8_percent_encode}; use serde::{Deserialize, Serialize}; use super::{ @@ -273,7 +273,7 @@ impl Skill { }; let name = self.name(); if name.contains(['/', '\\', '?', '#']) - || !percent_encoding::percent_decode_str(encoded_name).eq(name.bytes()) + || !percent_decode_str(encoded_name).eq(name.bytes()) { return Err(SkillValidationError::NameMismatch { uri: self.uri.clone(), @@ -328,7 +328,7 @@ fn manifest_resource_key(skill_uri: &str, resource_uri: &str) -> Option let root = skill_uri.strip_suffix(SKILL_MD)?; let relative = resource_uri.strip_prefix(root)?; let (path, suffix) = relative.split_at(relative.find(['?', '#']).unwrap_or(relative.len())); - let path = normalize_path_encoding(path); + let path = normalize_percent_encoding(path); let mut segments = path.split('/'); // An empty or dot-segment leaf resolves to a directory. let file = segments.next_back()?; @@ -353,10 +353,13 @@ fn manifest_resource_key(skill_uri: &str, resource_uri: &str) -> Option // RFC 3986 §6.2.2 decodes unreserved bytes and uppercases the hex digits in // other escapes. Reserved delimiters therefore stay encoded in the key. -fn normalize_path_encoding(path: &str) -> String { - let mut parts = path.split('%'); - let mut normalized = parts.next().unwrap_or_default().to_owned(); - for part in parts { +fn normalize_percent_encoding(value: &str) -> Cow<'_, str> { + let encoded: Cow<'_, str> = utf8_percent_encode(value, CONTROLS).into(); + let Some((prefix, escaped)) = encoded.split_once('%') else { + return encoded; + }; + let mut normalized = prefix.to_owned(); + for part in escaped.split('%') { if let Some((hex, rest)) = part.split_at_checked(2) && hex.bytes().all(|b| b.is_ascii_hexdigit()) { @@ -373,7 +376,7 @@ fn normalize_path_encoding(path: &str) -> String { normalized.push_str(part); } } - normalized + Cow::Owned(normalized) } /// Whether `digest` is `sha256:` followed by 64 lowercase hex characters. @@ -388,7 +391,7 @@ pub fn validate_skills(skills: &[Skill]) -> Result<(), SkillValidationError> { let mut seen = std::collections::HashSet::with_capacity(skills.len()); for skill in skills { skill.validate()?; - if !seen.insert(skill.uri.as_str()) { + if !seen.insert(normalize_percent_encoding(&skill.uri)) { return Err(SkillValidationError::DuplicateSkill(skill.uri.clone())); } } @@ -852,6 +855,28 @@ mod tests { Err(SkillValidationError::DuplicateSkill(_)) )); } + + #[test] + fn rejects_equivalent_percent_encodings_of_a_skill_uri() { + let skill = |uri| { + Skill::new( + uri, + SkillFrontmatter::new("résumé", "Résumé templates"), + SkillResources::Dynamic, + ) + }; + let uri = "skill://team/r%C3%A9sum%C3%A9/SKILL.md"; + for alias in [ + "skill://team/r%c3%a9sum%c3%a9/SKILL.md", + "skill://team/%72%C3%A9sum%C3%A9/SKILL.md", + "skill://team/résumé/SKILL.md", + ] { + assert_eq!( + validate_skills(&[skill(uri), skill(alias)]), + Err(SkillValidationError::DuplicateSkill(alias.into())) + ); + } + } } mod get_skill_result {