diff --git a/crates/zeph-skills/src/loader.rs b/crates/zeph-skills/src/loader.rs index 846b733a..385aeb5e 100644 --- a/crates/zeph-skills/src/loader.rs +++ b/crates/zeph-skills/src/loader.rs @@ -134,6 +134,37 @@ fn split_frontmatter(content: &str) -> Result<(&str, &str), SkillError> { Ok((yaml_str, body)) } +/// Verify that `path` resolves to a location inside `base_dir` after canonicalization. +/// +/// Prevents symlink-based path traversal by ensuring the canonical path +/// starts with the canonical base directory prefix. +/// +/// # Errors +/// +/// Returns `SkillError::Invalid` if the path escapes `base_dir`. +pub fn validate_path_within(path: &Path, base_dir: &Path) -> Result { + let canonical_base = base_dir.canonicalize().map_err(|e| { + SkillError::Other(format!( + "failed to canonicalize base dir {}: {e}", + base_dir.display() + )) + })?; + let canonical_path = path.canonicalize().map_err(|e| { + SkillError::Other(format!( + "failed to canonicalize path {}: {e}", + path.display() + )) + })?; + if !canonical_path.starts_with(&canonical_base) { + return Err(SkillError::Invalid(format!( + "path {} escapes skills directory {}", + canonical_path.display(), + canonical_base.display() + ))); + } + Ok(canonical_path) +} + /// Load only frontmatter metadata from a SKILL.md file. /// /// # Errors @@ -368,6 +399,36 @@ mod tests { assert!(load_skill_meta(&path).is_err()); } + #[test] + #[cfg(unix)] + fn validate_path_within_rejects_symlink_escape() { + let base = tempfile::tempdir().unwrap(); + let outside = tempfile::tempdir().unwrap(); + + let outside_file = outside.path().join("secret.txt"); + std::fs::write(&outside_file, "secret").unwrap(); + + let link_path = base.path().join("evil-link"); + std::os::unix::fs::symlink(&outside_file, &link_path).unwrap(); + let err = validate_path_within(&link_path, base.path()).unwrap_err(); + assert!( + format!("{err:#}").contains("escapes skills directory"), + "expected path traversal error, got: {err:#}" + ); + } + + #[test] + fn validate_path_within_accepts_legitimate_path() { + let base = tempfile::tempdir().unwrap(); + let inner = base.path().join("skill-dir"); + std::fs::create_dir_all(&inner).unwrap(); + let file = inner.join("SKILL.md"); + std::fs::write(&file, "content").unwrap(); + + let result = validate_path_within(&file, base.path()); + assert!(result.is_ok()); + } + #[test] fn name_validation_too_long() { let name = "a".repeat(65); diff --git a/crates/zeph-skills/src/registry.rs b/crates/zeph-skills/src/registry.rs index a1686e92..4511c1ae 100644 --- a/crates/zeph-skills/src/registry.rs +++ b/crates/zeph-skills/src/registry.rs @@ -4,7 +4,7 @@ use std::path::Path; use std::sync::OnceLock; use crate::error::SkillError; -use crate::loader::{Skill, SkillMeta, load_skill_body, load_skill_meta}; +use crate::loader::{Skill, SkillMeta, load_skill_body, load_skill_meta, validate_path_within}; struct SkillEntry { meta: SkillMeta, @@ -49,6 +49,10 @@ impl SkillRegistry { if !skill_path.is_file() { continue; } + if let Err(e) = validate_path_within(&skill_path, base) { + tracing::warn!("skipping skill path traversal: {e:#}"); + continue; + } match load_skill_meta(&skill_path) { Ok(meta) => { if seen.insert(meta.name.clone()) {