fix: contain and bound skill supporting file reads

crates/goose/Cargo.toml

@@ -236,6 +236,7 @@ gethostname = "1.1.0"
 [target.'cfg(target_os = "windows")'.dependencies]
 winapi = { workspace = true, features = ["accctrl", "aclapi", "fileapi", "handleapi", "minwinbase", "sddl", "securitybaseapi", "winbase", "winerror"] }
 keyring = { workspace = true, features = ["windows-native"], optional = true }
+ntapi = { version = "0.4.3", default-features = false }
 
 # Platform-specific GPU acceleration for Whisper and local inference
 [target.'cfg(target_os = "macos")'.dependencies]

crates/goose/src/agents/large_response_handler.rs

@@ -4,7 +4,7 @@ use std::io::Write;
 
 const DEFAULT_LARGE_TEXT_THRESHOLD: usize = 200_000;
 
-fn large_text_threshold() -> usize {
+pub(crate) fn max_tool_response_size() -> usize {
     Config::global()
         .get_param::<usize>("GOOSE_MAX_TOOL_RESPONSE_SIZE")
         .unwrap_or(DEFAULT_LARGE_TEXT_THRESHOLD)
@@ -14,7 +14,7 @@ fn large_text_threshold() -> usize {
 pub fn process_tool_response(
     response: Result<CallToolResult, ErrorData>,
 ) -> Result<CallToolResult, ErrorData> {
-    let threshold = large_text_threshold();
+    let threshold = max_tool_response_size();
     match response {
         Ok(mut result) => {
             let mut processed_contents = Vec::new();

crates/goose/src/agents/mod.rs

@@ -31,6 +31,7 @@ pub use execute_commands::{context_management_unsupported_message, COMPACT_TRIGG
 pub use extension::{ExtensionConfig, ExtensionError};
 pub use extension_manager::ExtensionManager;
 pub use goose_agent::events::AgentEvent;
+pub(crate) use large_response_handler::max_tool_response_size;
 pub use prompt_manager::PromptManager;
 pub use schedule_tool::ScheduleTool;
 pub use subagent_handler::SUBAGENT_TOOL_REQUEST_TYPE;

crates/goose/src/agents/platform_extensions/summon.rs

@@ -197,6 +197,11 @@ fn parse_agent_content(content: &str, path: &Path) -> Option<SourceEntry> {
         format!("Agent{}", model_info)
     });
 
+    let mut properties = std::collections::HashMap::new();
+    if let Some(model) = metadata.model {
+        properties.insert("model".to_string(), serde_json::Value::String(model));
+    }
+
     Some(SourceEntry {
         source_type: SourceType::Agent,
         name: metadata.name,
@@ -206,7 +211,7 @@ fn parse_agent_content(content: &str, path: &Path) -> Option<SourceEntry> {
         global: false,
         writable: true,
         supporting_files: Vec::new(),
-        properties: std::collections::HashMap::new(),
+        properties,
     })
 }
 
@@ -217,16 +222,17 @@ fn scan_recipes_from_dir(
     sources: &mut Vec<SourceEntry>,
     seen: &mut std::collections::HashSet<String>,
 ) {
-    let entries = match std::fs::read_dir(dir) {
+    let Ok(source_dir) = dir.canonicalize() else {
+        return;
+    };
+    let entries = match std::fs::read_dir(&source_dir) {
         Ok(e) => e,
         Err(_) => return,
     };
 
     for entry in entries.flatten() {
-        let path = entry.path();
-        if !path.is_file() {
-            continue;
-        }
+        let file_name = entry.file_name();
+        let path = source_dir.join(&file_name);
 
         let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("");
         if !RECIPE_FILE_EXTENSIONS.contains(&ext) {
@@ -243,7 +249,19 @@ fn scan_recipes_from_dir(
             continue;
         }
 
-        match Recipe::from_file_path(&path) {
+        let content = match crate::skills::read_source_file_with_limit(
+            &source_dir,
+            Path::new(&file_name),
+            crate::agents::max_tool_response_size(),
+        ) {
+            Ok(content) => content,
+            Err(error) => {
+                warn!("Failed to read recipe {}: {}", path.display(), error);
+                continue;
+            }
+        };
+
+        match Recipe::from_content(&content) {
             Ok(recipe) => {
                 seen.insert(name.clone());
                 sources.push(SourceEntry {
@@ -277,23 +295,28 @@ fn scan_agents_from_dir(
     sources: &mut Vec<SourceEntry>,
     seen: &mut std::collections::HashSet<String>,
 ) {
-    let entries = match std::fs::read_dir(dir) {
+    let Ok(source_dir) = dir.canonicalize() else {
+        return;
+    };
+    let entries = match std::fs::read_dir(&source_dir) {
         Ok(e) => e,
         Err(_) => return,
     };
 
     for entry in entries.flatten() {
-        let path = entry.path();
-        if !path.is_file() {
-            continue;
-        }
+        let file_name = entry.file_name();
+        let path = source_dir.join(&file_name);
 
         let ext = path.extension().and_then(|e| e.to_str()).unwrap_or("");
         if ext != "md" {
             continue;
         }
 
-        let content = match std::fs::read_to_string(&path) {
+        let content = match crate::skills::read_source_file_with_limit(
+            &source_dir,
+            Path::new(&file_name),
+            crate::agents::max_tool_response_size(),
+        ) {
             Ok(c) => c,
             Err(e) => {
                 warn!("Failed to read agent file {}: {}", path.display(), e);
@@ -1575,18 +1598,15 @@ impl SummonClient {
         source: &SourceEntry,
         params: &DelegateParams,
     ) -> Result<Recipe, String> {
-        let agent_content = if source.path.is_empty() {
+        if source.path.is_empty() {
             return Err("Agent source has no path".to_string());
-        } else {
-            std::fs::read_to_string(&source.path)
-                .map_err(|e| format!("Failed to read agent file: {}", e))?
-        };
-
-        let (metadata, _): (AgentMetadata, String) = parse_frontmatter(&agent_content)
-            .map_err(|e| format!("Failed to parse agent frontmatter: {}", e))?
-            .ok_or("No frontmatter found in agent file")?;
+        }
 
-        let model = metadata.model;
+        let model = source
+            .properties
+            .get("model")
+            .and_then(serde_json::Value::as_str)
+            .map(str::to_string);
 
         // max_turns is set later in build_task_config so it can incorporate params.max_turns
         // with the correct priority ordering; setting it here would cause it to be overridden
@@ -2246,6 +2266,13 @@ You review code."#;
         let source = parse_agent_content(agent, Path::new("")).unwrap();
         assert_eq!(source.name, "reviewer");
         assert!(source.description.contains("sonnet"));
+        assert_eq!(
+            source
+                .properties
+                .get("model")
+                .and_then(|value| value.as_str()),
+            Some("sonnet")
+        );
     }
 
     #[test]
@@ -2335,6 +2362,29 @@ You review code."#;
         assert_eq!(sources[0].name, "reviewer");
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn agent_scan_rejects_symlinked_source_file() {
+        let temp_dir = TempDir::new().unwrap();
+        let outside = TempDir::new().unwrap();
+        fs::write(
+            outside.path().join("outside.md"),
+            "---\nname: outside\n---\nUntrusted agent.",
+        )
+        .unwrap();
+        std::os::unix::fs::symlink(
+            outside.path().join("outside.md"),
+            temp_dir.path().join("outside.md"),
+        )
+        .unwrap();
+
+        let mut sources = Vec::new();
+        let mut seen = HashSet::new();
+        scan_agents_from_dir(temp_dir.path(), &mut sources, &mut seen);
+
+        assert!(sources.is_empty());
+    }
+
     #[test]
     fn test_recipe_scan_skips_non_recipe_project_config_files() {
         let temp_dir = TempDir::new().unwrap();
@@ -2369,6 +2419,35 @@ You review code."#;
         assert_eq!(sources[0].description, "Real recipe");
     }
 
+    #[cfg(unix)]
+    #[test]
+    fn recipe_scan_rejects_symlinked_source_file() {
+        let temp_dir = TempDir::new().unwrap();
+        let outside = TempDir::new().unwrap();
+        fs::write(
+            outside.path().join("outside.yaml"),
+            "title: Outside\ndescription: Outside recipe\ninstructions: Untrusted",
+        )
+        .unwrap();
+        std::os::unix::fs::symlink(
+            outside.path().join("outside.yaml"),
+            temp_dir.path().join("outside.yaml"),
+        )
+        .unwrap();
+
+        let mut sources = Vec::new();
+        let mut seen = HashSet::new();
+        scan_recipes_from_dir(
+            temp_dir.path(),
+            SourceType::Recipe,
+            false,
+            &mut sources,
+            &mut seen,
+        );
+
+        assert!(sources.is_empty());
+    }
+
     #[tokio::test]
     async fn test_discover_recipes_and_agents() {
         let temp_dir = TempDir::new().unwrap();

crates/goose/src/agents/state_machine/ops_skills.rs

@@ -146,9 +146,6 @@ fn load_supporting_file(
     relative_path: &str,
 ) -> CallToolResult {
     let skill_dir = PathBuf::from(&skill.path);
-    let canonical_skill_dir = skill_dir
-        .canonicalize()
-        .unwrap_or_else(|_| skill_dir.clone());
     for file_path in &skill.supporting_files {
         let file_path = Path::new(file_path);
         let Ok(relative) = file_path.strip_prefix(&skill_dir) else {
@@ -157,22 +154,10 @@ fn load_supporting_file(
         if relative.to_string_lossy().replace('\\', "/") != relative_path {
             continue;
         }
-        return match file_path.canonicalize() {
-            Ok(canonical) if canonical.starts_with(&canonical_skill_dir) => {
-                match std::fs::read_to_string(&canonical) {
-                    Ok(content) => CallToolResult::success(vec![ContentBlock::text(format!(
-                        "# Loaded: {skill_name}\n\n{content}\n\n---\nFile loaded into context."
-                    ))]),
-                    Err(error) => CallToolResult::error(vec![ContentBlock::text(format!(
-                        "Failed to read '{skill_name}': {error}"
-                    ))]),
-                }
-            }
-            Ok(_) => CallToolResult::error(vec![ContentBlock::text(format!(
-                "Refusing to load '{skill_name}': resolves outside the skill directory"
-            ))]),
+        return match crate::skills::load_supporting_file(&skill_dir, relative, skill_name) {
+            Ok(content) => CallToolResult::success(vec![ContentBlock::text(content)]),
             Err(error) => CallToolResult::error(vec![ContentBlock::text(format!(
-                "Failed to resolve '{skill_name}': {error}"
+                "Failed to read '{skill_name}': {error}"
             ))]),
         };
     }
@@ -360,3 +345,36 @@ impl Operation<Session, GooseEffect> for SkillOperation {
         applied([response.into()])
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::collections::HashMap;
+
+    #[test]
+    fn supporting_file_loader_reads_nested_regular_file() {
+        let root = tempfile::tempdir().unwrap();
+        let skill_dir = std::fs::canonicalize(root.path()).unwrap();
+        let nested = skill_dir.join("nested");
+        std::fs::create_dir(&nested).unwrap();
+        let file = nested.join("guide.md");
+        std::fs::write(&file, "Nested guidance.").unwrap();
+        let skill = SourceEntry {
+            source_type: SourceType::Skill,
+            name: "test-skill".to_string(),
+            description: String::new(),
+            content: String::new(),
+            path: skill_dir.to_string_lossy().into_owned(),
+            global: false,
+            writable: true,
+            supporting_files: vec![file.to_string_lossy().into_owned()],
+            properties: HashMap::new(),
+        };
+
+        let result = load_supporting_file(&skill, "test-skill/nested/guide.md", "nested/guide.md");
+
+        assert_eq!(result.is_error, Some(false));
+        let text = result.content[0].as_text().expect("expected text");
+        assert!(text.text.contains("Nested guidance."));
+    }
+}

crates/goose/src/recipe/read_recipe_file_content.rs

@@ -12,27 +12,32 @@ pub struct RecipeFile {
 pub fn read_recipe_file<P: AsRef<Path>>(recipe_path: P) -> Result<RecipeFile> {
     let raw_path = recipe_path.as_ref();
     let path = convert_path_with_tilde_expansion(raw_path);
-
-    let content = fs::read_to_string(&path)
-        .map_err(|e| anyhow!("Failed to read recipe file {}: {}", path.display(), e))?;
-
-    let canonical = path.canonicalize().map_err(|e| {
+    let parent = path
+        .parent()
+        .filter(|parent| !parent.as_os_str().is_empty())
+        .unwrap_or_else(|| Path::new("."));
+    let parent_dir = parent.canonicalize().map_err(|e| {
         anyhow!(
-            "Failed to resolve absolute path for {}: {}",
-            path.display(),
+            "Failed to resolve recipe directory {}: {}",
+            parent.display(),
             e
         )
     })?;
-
-    let parent_dir = canonical
-        .parent()
-        .ok_or_else(|| anyhow!("Resolved path has no parent: {}", canonical.display()))?
-        .to_path_buf();
+    let file_name = path
+        .file_name()
+        .ok_or_else(|| anyhow!("Recipe path has no file name: {}", path.display()))?;
+    let content = crate::skills::read_source_file_with_limit(
+        &parent_dir,
+        Path::new(file_name),
+        crate::agents::max_tool_response_size(),
+    )
+    .map_err(|e| anyhow!("Failed to read recipe file {}: {}", path.display(), e))?;
+    let file_path = parent_dir.join(file_name);
 
     Ok(RecipeFile {
         content,
         parent_dir,
-        file_path: canonical,
+        file_path,
     })
 }
 
@@ -99,4 +104,21 @@ mod tests {
             .to_string()
             .contains("Failed to read parameter file"));
     }
+
+    #[cfg(unix)]
+    #[test]
+    fn read_recipe_file_rejects_symlink() {
+        let temp_dir = TempDir::new().unwrap();
+        let outside = TempDir::new().unwrap();
+        let outside_recipe = outside.path().join("outside.yaml");
+        std::fs::write(
+            &outside_recipe,
+            "title: Outside\ndescription: Outside\ninstructions: Untrusted",
+        )
+        .unwrap();
+        let linked_recipe = temp_dir.path().join("linked.yaml");
+        std::os::unix::fs::symlink(outside_recipe, &linked_recipe).unwrap();
+
+        assert!(read_recipe_file(linked_recipe).is_err());
+    }
 }