From df6c3c50dbe580df294b415b157a38f4d7e8b47f Mon Sep 17 00:00:00 2001
From: Alex Clarke <alex.j.tusa@gmail.com>
Date: Wed, 3 Jun 2026 14:59:57 -0600
Subject: [PATCH] fix: added path validation for skill names

---
 src/config/paths.rs           | 54 +++++++++++++++++++++++++++++++++++
 src/config/request_context.rs |  2 ++
 src/config/skill.rs           |  1 +
 3 files changed, 57 insertions(+)

diff --git a/src/config/paths.rs b/src/config/paths.rs
index 768e26d..9c637dd 100644
--- a/src/config/paths.rs
+++ b/src/config/paths.rs
@@ -80,6 +80,21 @@ pub fn skill_file(name: &str) -> PathBuf {
     skill_dir(name).join("SKILL.md")
 }
 
+pub fn validate_skill_name(name: &str) -> Result<()> {
+    if name.is_empty() {
+        bail!("Skill name cannot be empty");
+    }
+    if !name
+        .chars()
+        .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_')
+    {
+        bail!(
+            "Invalid skill name '{name}': only letters, digits, '-', and '_' are allowed"
+        );
+    }
+    Ok(())
+}
+
 pub fn macros_dir() -> PathBuf {
     match env::var(get_env_name("macros_dir")) {
         Ok(value) => PathBuf::from(value),
@@ -286,3 +301,42 @@ pub fn local_models_override() -> Result<Vec<ProviderModels>> {
     }
     Ok(models_override.list)
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn validate_skill_name_accepts_alphanumerics_and_dashes() {
+        assert!(validate_skill_name("git-master").is_ok());
+        assert!(validate_skill_name("code_review").is_ok());
+        assert!(validate_skill_name("Skill1").is_ok());
+    }
+
+    #[test]
+    fn validate_skill_name_rejects_empty() {
+        let err = validate_skill_name("").unwrap_err();
+        assert!(err.to_string().contains("cannot be empty"));
+    }
+
+    #[test]
+    fn validate_skill_name_rejects_path_traversal() {
+        for bad in ["../escape", "..", "foo/bar", "foo\\bar", "./hidden"] {
+            let err = validate_skill_name(bad).unwrap_err();
+            assert!(
+                err.to_string().contains("Invalid skill name"),
+                "expected rejection for {bad:?}, got: {err}"
+            );
+        }
+    }
+
+    #[test]
+    fn validate_skill_name_rejects_other_special_chars() {
+        for bad in ["with space", "null\0byte", "weird?char", "dot.name"] {
+            assert!(
+                validate_skill_name(bad).is_err(),
+                "expected rejection for {bad:?}"
+            );
+        }
+    }
+}
diff --git a/src/config/request_context.rs b/src/config/request_context.rs
index 4d6af34..bf63b65 100644
--- a/src/config/request_context.rs
+++ b/src/config/request_context.rs
@@ -2584,6 +2584,7 @@ impl RequestContext {
     }
 
     pub fn upsert_skill(&self, app: &AppConfig, name: &str) -> Result<()> {
+        paths::validate_skill_name(name)?;
         let path = paths::skill_file(name);
         ensure_parent_exists(&path)?;
         let is_new = !path.exists();
@@ -2602,6 +2603,7 @@ impl RequestContext {
     }
 
     pub async fn load_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> {
+        paths::validate_skill_name(name)?;
         if !self.app.config.function_calling_support {
             bail!(
                 "Skills require function calling, which is disabled. Enable function calling in your config then try again."
diff --git a/src/config/skill.rs b/src/config/skill.rs
index 0ead4b4..b0f3837 100644
--- a/src/config/skill.rs
+++ b/src/config/skill.rs
@@ -116,6 +116,7 @@ impl Skill {
     }
 
     pub fn load(name: &str) -> Result<Self> {
+        paths::validate_skill_name(name)?;
         let path = paths::skill_file(name);
         let content = read_to_string(&path)
             .with_context(|| format!("Failed to read skill '{name}' at {}", path.display()))?;