fix(skills): support legacy allowed-tools formats

Author: Lixtt

core/src/skills/mod.rs

@@ -33,7 +33,7 @@ pub use validator::{
     DefaultSkillValidator, SkillValidationError, SkillValidator, ValidationErrorKind,
 };
 
-use serde::{Deserialize, Serialize};
+use serde::{de, Deserialize, Deserializer, Serialize};
 use std::collections::HashSet;
 use std::path::Path;
 
@@ -101,7 +101,11 @@ pub struct Skill {
     pub description: String,
 
     /// Allowed tools (Claude Code format: "Bash(pattern:*), read(*)")
-    #[serde(default, rename = "allowed-tools")]
+    #[serde(
+        default,
+        rename = "allowed-tools",
+        deserialize_with = "deserialize_allowed_tools"
+    )]
     pub allowed_tools: Option<String>,
 
     /// Whether to disable model invocation
@@ -183,11 +187,25 @@ impl Skill {
             return permissions;
         };
 
-        // Parse comma-separated tool permissions
-        for part in allowed.split(',') {
+        // Parse Claude-style comma-separated permissions, plus legacy
+        // whitespace-only tool lists such as "Read Write Edit Bash".
+        let parts: Vec<&str> = if allowed.contains(',') {
+            allowed.split(',').collect()
+        } else {
+            allowed.split_whitespace().collect()
+        };
+        for part in parts {
             let part = part.trim();
+            if part.is_empty() {
+                continue;
+            }
             if let Some(perm) = ToolPermission::parse(part) {
                 permissions.insert(perm);
+            } else {
+                permissions.insert(ToolPermission {
+                    tool: part.to_string(),
+                    pattern: "*".to_string(),
+                });
             }
         }
 
@@ -218,6 +236,34 @@ impl Skill {
     }
 }
 
+fn deserialize_allowed_tools<'de, D>(deserializer: D) -> Result<Option<String>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let value = Option::<serde_yaml::Value>::deserialize(deserializer)?;
+    match value {
+        None | Some(serde_yaml::Value::Null) => Ok(None),
+        Some(serde_yaml::Value::String(s)) => Ok(Some(s)),
+        Some(serde_yaml::Value::Sequence(items)) => {
+            let mut tools = Vec::new();
+            for item in items {
+                match item {
+                    serde_yaml::Value::String(s) => tools.push(s),
+                    other => {
+                        return Err(de::Error::custom(format!(
+                            "allowed-tools list entries must be strings, got {other:?}"
+                        )));
+                    }
+                }
+            }
+            Ok(Some(tools.join(", ")))
+        }
+        Some(other) => Err(de::Error::custom(format!(
+            "allowed-tools must be a string or a list of strings, got {other:?}"
+        ))),
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -270,6 +316,51 @@ You are a test assistant.
         assert_eq!(permissions.len(), 3);
     }
 
+    #[test]
+    fn test_parse_legacy_whitespace_allowed_tools() {
+        let skill = Skill {
+            name: "test".to_string(),
+            description: "test".to_string(),
+            allowed_tools: Some("Read Write Edit Bash".to_string()),
+            disable_model_invocation: false,
+            kind: SkillKind::Instruction,
+            content: String::new(),
+            tags: Vec::new(),
+            version: None,
+        };
+
+        let permissions = skill.parse_allowed_tools();
+        assert_eq!(permissions.len(), 4);
+        assert!(permissions
+            .iter()
+            .any(|perm| perm.tool == "Bash" && perm.pattern == "*"));
+    }
+
+    #[test]
+    fn test_parse_allowed_tools_yaml_list() {
+        let content = r#"---
+name: test-skill
+description: A test skill
+allowed-tools:
+  - Read
+  - Write
+  - Bash(uv run skills analyze-ci:*)
+---
+# Instructions
+"#;
+
+        let skill = Skill::parse(content).unwrap();
+        assert_eq!(
+            skill.allowed_tools.as_deref(),
+            Some("Read, Write, Bash(uv run skills analyze-ci:*)")
+        );
+        let permissions = skill.parse_allowed_tools();
+        assert_eq!(permissions.len(), 3);
+        assert!(permissions
+            .iter()
+            .any(|perm| perm.tool == "Read" && perm.pattern == "*"));
+    }
+
     #[test]
     fn test_is_tool_allowed() {
         let skill = Skill {

core/src/tools/skill.rs

@@ -226,6 +226,16 @@ impl SkillTool {
     fn create_skill_permission_policy(skill: &Skill) -> PermissionPolicy {
         let permissions = skill.parse_allowed_tools();
 
+        if permissions.is_empty() {
+            return PermissionPolicy {
+                deny: Vec::new(),
+                allow: Vec::new(),
+                ask: Vec::new(),
+                default_decision: PermissionDecision::Allow,
+                enabled: true,
+            };
+        }
+
         // Convert skill permissions to PermissionRules
         let mut allow_rules = Vec::new();
         for perm in permissions {
@@ -453,6 +463,56 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_skill_permission_policy_allows_when_unspecified() {
+        let skill = Skill {
+            name: "test-skill".to_string(),
+            description: "Test".to_string(),
+            allowed_tools: None,
+            disable_model_invocation: false,
+            kind: SkillKind::Instruction,
+            content: String::new(),
+            tags: Vec::new(),
+            version: None,
+        };
+
+        let policy = SkillTool::create_skill_permission_policy(&skill);
+
+        assert_eq!(
+            policy.check("bash", &serde_json::json!({"command": "python --version"})),
+            PermissionDecision::Allow
+        );
+        assert_eq!(
+            policy.check("read", &serde_json::json!({"file_path": "SKILL.md"})),
+            PermissionDecision::Allow
+        );
+    }
+
+    #[test]
+    fn test_skill_permission_policy_accepts_legacy_allowed_tools() {
+        let skill = Skill {
+            name: "test-skill".to_string(),
+            description: "Test".to_string(),
+            allowed_tools: Some("Read Write Edit Bash".to_string()),
+            disable_model_invocation: false,
+            kind: SkillKind::Instruction,
+            content: String::new(),
+            tags: Vec::new(),
+            version: None,
+        };
+
+        let policy = SkillTool::create_skill_permission_policy(&skill);
+
+        assert_eq!(
+            policy.check("bash", &serde_json::json!({"command": "python --version"})),
+            PermissionDecision::Allow
+        );
+        assert_eq!(
+            policy.check("grep", &serde_json::json!({"pattern": "x"})),
+            PermissionDecision::Deny
+        );
+    }
+
     #[test]
     fn test_skill_args_accepts_documented_shape() {
         let args =