Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 61 additions & 0 deletions crates/zeph-skills/src/loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf, SkillError> {
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
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 5 additions & 1 deletion crates/zeph-skills/src/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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()) {
Expand Down
Loading