tkmind/tkmind_go
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

feat(summon): make skill supporting files individually loadable via load() (#7583)

Browse Source
This commit is contained in:
Will Pfleger
2026-03-09 18:03:52 -04:00
committed by GitHub
parent ff4a8595e6
commit c1d8e27e73
2 changed files with 329 additions and 36 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose/src/agents/platform_extensions/summon.rs
+328 -9
View File
@@ -169,6 +169,14 @@ fn parse_frontmatter<T: for<'de> Deserialize<'de>>(content: &str) -> Option<(T,
fn parse_skill_content(content: &str, path: PathBuf) -> Option<Source> {
let (metadata, body): (SkillMetadata, String) = parse_frontmatter(content)?;
if metadata.name.contains('/') {
warn!(
"Skill name '{}' contains '/' which is not allowed, skipping",
metadata.name
);
return None;
}
Some(Source {
name: metadata.name,
kind: SourceKind::Skill,
@@ -693,15 +701,58 @@ impl SummonClient {
session_id: &str,
name: &str,
working_dir: &Path,
) -> Option<Source> {
) -> Result<Option<Source>, String> {
let sources = self.get_sources(session_id, working_dir).await;
let mut source = sources.into_iter().find(|s| s.name == name)?;
if source.kind == SourceKind::Subrecipe && source.content.is_empty() {
source.content = self.load_subrecipe_content(session_id, &source.name).await;
if let Some(mut source) = sources.iter().find(|s| s.name == name).cloned() {
if source.kind == SourceKind::Subrecipe && source.content.is_empty() {
source.content = self.load_subrecipe_content(session_id, &source.name).await;
}
return Ok(Some(source));
}
Some(source)
if let Some((skill_name, raw_relative_path)) = name.split_once('/') {
let relative_path = raw_relative_path.replace('\\', "/");
if let Some(skill) = sources.iter().find(|s| {
s.name == skill_name
&& matches!(s.kind, SourceKind::Skill | SourceKind::BuiltinSkill)
}) {
let canonical_skill_dir = skill
.path
.canonicalize()
.unwrap_or_else(|_| skill.path.clone());
for file_path in &skill.supporting_files {
if let Ok(rel) = file_path.strip_prefix(&skill.path) {
let rel_normalized = rel.to_string_lossy().replace('\\', "/");
if rel_normalized == relative_path {
let canonical_file = file_path
.canonicalize()
.map_err(|e| format!("Failed to resolve '{}': {}", name, e))?;
if !canonical_file.starts_with(&canonical_skill_dir) {
return Err(format!(
"Refusing to load '{}': file resolves outside the skill directory",
name
));
}
return match std::fs::read_to_string(&canonical_file) {
Ok(content) => Ok(Some(Source {
name: name.to_string(),
kind: SourceKind::Skill,
description: format!("Supporting file for {}", skill_name),
path: file_path.clone(),
content,
supporting_files: vec![],
})),
Err(e) => Err(format!("Failed to read '{}': {}", name, e)),
};
}
}
}
}
}
Ok(None)
}
async fn load_subrecipe_content(&self, session_id: &str, name: &str) -> String {
@@ -1041,7 +1092,7 @@ impl SummonClient {
name: &str,
working_dir: &Path,
) -> Result<Vec<Content>, String> {
let source = self.resolve_source(session_id, name, working_dir).await;
let source = self.resolve_source(session_id, name, working_dir).await?;
match source {
Some(source) => {
@@ -1059,11 +1110,15 @@ impl SummonClient {
));
for file in &source.supporting_files {
if let Ok(relative) = file.strip_prefix(&source.path) {
output.push_str(&format!("- {}\n", relative.display()));
let rel_str = relative.to_string_lossy().replace('\\', "/");
output.push_str(&format!(
"- {} → load(source: \"{}/{}\")\n",
rel_str, source.name, rel_str
));
}
}
output.push_str(
"\nUse the file tools to read these files or run scripts as directed.\n",
"\nUse load(source: \"<skill-name>/<path>\") to load individual files into context, or use file tools to read/run them directly.\n",
);
}
@@ -1073,6 +1128,42 @@ impl SummonClient {
}
None => {
let sources = self.get_sources(session_id, working_dir).await;
if let Some((skill_name, _)) = name.split_once('/') {
if let Some(skill) = sources.iter().find(|s| {
s.name == skill_name
&& matches!(s.kind, SourceKind::Skill | SourceKind::BuiltinSkill)
}) {
let available: Vec<String> = skill
.supporting_files
.iter()
.filter_map(|f| {
f.strip_prefix(&skill.path)
.ok()
.map(|r| r.to_string_lossy().replace('\\', "/"))
})
.collect();
if !available.is_empty() {
let total = available.len();
let display: Vec<_> = available.into_iter().take(10).collect();
let suffix = if total > 10 {
format!(" (and {} more)", total - 10)
} else {
String::new()
};
return Err(format!(
"Source '{}' not found. Available files for {}: {}{}",
name,
skill_name,
display.join(", "),
suffix
));
} else {
return Err(format!("Skill '{}' has no supporting files.", skill_name));
}
}
}
let suggestions: Vec<&str> = sources
.iter()
.filter(|s| {
@@ -1244,9 +1335,18 @@ impl SummonClient {
) -> Result<Recipe, String> {
let source = self
.resolve_source(session_id, source_name, working_dir)
.await
.await?
.ok_or_else(|| format!("Source '{}' not found", source_name))?;
if source_name.contains('/')
&& matches!(source.kind, SourceKind::Skill | SourceKind::BuiltinSkill)
{
return Err(format!(
"Cannot delegate to supporting file '{}'. Use load() to read it instead.",
source_name
));
}
let mut recipe = match source.kind {
SourceKind::Recipe | SourceKind::Subrecipe => {
self.build_recipe_from_source(&source, params, session_id)
@@ -1922,6 +2022,225 @@ You review code."#;
assert!(file_names.contains(&"report.txt".to_string()));
}
#[tokio::test]
async fn test_load_source_lists_supporting_files_with_load_names() {
let temp_dir = TempDir::new().unwrap();
let skill_dir = temp_dir.path().join(".goose/skills/my-skill");
fs::create_dir_all(skill_dir.join("references")).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
"---\nname: my-skill\ndescription: A skill\n---\nSee references.",
)
.unwrap();
fs::write(
skill_dir.join("references/ops.md"),
"# Ops Guide\n\nDo the thing.",
)
.unwrap();
fs::write(skill_dir.join("run.sh"), "#!/bin/bash\necho ok").unwrap();
let client = SummonClient::new(create_test_context()).unwrap();
let result = client
.handle_load_source("test", "my-skill", temp_dir.path())
.await
.unwrap();
let text = &result[0].as_text().expect("expected text content").text;
assert!(
!text.contains("Ops Guide"),
"md file content should not be inlined"
);
assert!(
!text.contains("Do the thing."),
"md file content should not be inlined"
);
assert!(
!text.contains("#!/bin/bash"),
"script content should not be inlined"
);
assert!(
text.contains("load(source: \"my-skill/references/ops.md\")"),
"md file should be listed with load() name"
);
assert!(
text.contains("load(source: \"my-skill/run.sh\")"),
"script should be listed with load() name"
);
assert!(
text.contains("load(source: \"<skill-name>/<path>\")"),
"should include usage hint"
);
}
#[tokio::test]
async fn test_load_supporting_file_by_path() {
let temp_dir = TempDir::new().unwrap();
let skill_dir = temp_dir.path().join(".goose/skills/my-skill");
fs::create_dir_all(skill_dir.join("references")).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
"---\nname: my-skill\ndescription: A skill\n---\nSee references.",
)
.unwrap();
fs::write(
skill_dir.join("references/ops.md"),
"# Ops Guide\n\nDo the thing.",
)
.unwrap();
fs::write(skill_dir.join("run.sh"), "#!/bin/bash\necho ok").unwrap();
let client = SummonClient::new(create_test_context()).unwrap();
let md_result = client
.handle_load_source("test", "my-skill/references/ops.md", temp_dir.path())
.await
.unwrap();
let md_text = &md_result[0].as_text().expect("expected text content").text;
assert!(
md_text.contains("Ops Guide"),
"markdown content should be loaded"
);
assert!(md_text.contains("Do the thing."));
let sh_result = client
.handle_load_source("test", "my-skill/run.sh", temp_dir.path())
.await
.unwrap();
let sh_text = &sh_result[0].as_text().expect("expected text content").text;
assert!(
sh_text.contains("#!/bin/bash"),
"script content should be loaded"
);
}
#[tokio::test]
async fn test_load_supporting_file_not_found_suggests_available() {
let temp_dir = TempDir::new().unwrap();
let skill_dir = temp_dir.path().join(".goose/skills/my-skill");
fs::create_dir_all(skill_dir.join("references")).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
"---\nname: my-skill\ndescription: A skill\n---\nSee references.",
)
.unwrap();
fs::write(
skill_dir.join("references/ops.md"),
"# Ops Guide\n\nDo the thing.",
)
.unwrap();
let client = SummonClient::new(create_test_context()).unwrap();
let err = client
.handle_load_source(
"test",
"my-skill/references/nonexistent.md",
temp_dir.path(),
)
.await
.unwrap_err();
assert!(
err.contains("references/ops.md"),
"error should list available files: {}",
err
);
assert!(
err.contains("my-skill"),
"error should name the skill: {}",
err
);
}
#[cfg(unix)]
#[tokio::test]
async fn test_resolve_source_blocks_symlink_outside_skill_dir() {
let temp_dir = TempDir::new().unwrap();
let outside_dir = TempDir::new().unwrap();
let skill_dir = temp_dir.path().join(".goose/skills/my-skill");
fs::create_dir_all(&skill_dir).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
"---\nname: my-skill\ndescription: A skill\n---\nContent.",
)
.unwrap();
let secret_file = outside_dir.path().join("secret.txt");
fs::write(&secret_file, "top secret data").unwrap();
std::os::unix::fs::symlink(&secret_file, skill_dir.join("evil.md")).unwrap();
let client = SummonClient::new(create_test_context()).unwrap();
let result = client
.handle_load_source("test", "my-skill/evil.md", temp_dir.path())
.await;
assert!(
result.is_err(),
"symlink outside skill dir should be blocked"
);
let err = result.unwrap_err();
assert!(
err.contains("resolves outside the skill directory"),
"error should mention path traversal: {}",
err
);
}
#[tokio::test]
async fn test_resolve_source_blocks_path_traversal_input() {
let temp_dir = TempDir::new().unwrap();
let skill_dir = temp_dir.path().join(".goose/skills/my-skill");
fs::create_dir_all(&skill_dir).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
"---\nname: my-skill\ndescription: A skill\n---\nContent.",
)
.unwrap();
fs::write(skill_dir.join("legit.md"), "legit content").unwrap();
let client = SummonClient::new(create_test_context()).unwrap();
// ../../../etc/passwd won't match any supporting_files entry, so it returns Ok (not found)
// which becomes the "not found" error path in handle_load_source
let result = client
.handle_load_source("test", "my-skill/../../../etc/passwd", temp_dir.path())
.await;
assert!(result.is_err(), "traversal path should not load content");
let err = result.unwrap_err();
assert!(
!err.contains("root:"),
"should not contain /etc/passwd content: {}",
err
);
}
#[tokio::test]
async fn test_skill_name_with_slash_is_rejected() {
let temp_dir = TempDir::new().unwrap();
let skill_dir = temp_dir.path().join(".goose/skills/bad-skill");
fs::create_dir_all(&skill_dir).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
"---\nname: bad/skill\ndescription: A skill with slash\n---\nContent.",
)
.unwrap();
let client = SummonClient::new(create_test_context()).unwrap();
let sources = client.get_sources("test", temp_dir.path()).await;
assert!(
!sources.iter().any(|s| s.name == "bad/skill"),
"skill with '/' in name should be rejected"
);
}
#[tokio::test]
async fn test_client_tools_and_unknown_tool() {
let client = SummonClient::new(create_test_context()).unwrap();
ui/desktop/package-lock.json
Generated
+1 -27
View File
@@ -123,7 +123,7 @@
}
},
"../acp": {
"name": "goose-acp-types",
"name": "@block/goose-acp",
"version": "0.1.0",
"dependencies": {
"zod": "^3.25.76"
@@ -10330,32 +10330,6 @@
"node": ">= 0.8"
}
},
"node_modules/encoding": {
"version": "0.1.13",
"resolved": "https://registry.npmjs.org/encoding/-/encoding-0.1.13.tgz",
"integrity": "sha512-ETBauow1T35Y/WZMkio9jiM0Z5xjHHmJ4XmjZOq1l/dXz3lr2sRn87nJy20RupqSh1F2m3HHPSp8ShIPQJrJ3A==",
"dev": true,
"license": "MIT",
"optional": true,
"peer": true,
"dependencies": {
"iconv-lite": "^0.6.2"
}
},
"node_modules/encoding/node_modules/iconv-lite": {
"version": "0.6.3",
"resolved": "https://registry.npmjs.org/iconv-lite/-/iconv-lite-0.6.3.tgz",
"integrity": "sha512-4fCk79wshMdzMp2rH06qWrJE4iolqLhCUH+OiuIgU++RB0+94NlDL81atO7GX55uUKueo0txHNtvEyI6D7WdMw==",
"dev": true,
"license": "MIT",
"optional": true,
"dependencies": {
"safer-buffer": ">= 2.1.2 < 3.0.0"
},
"engines": {
"node": ">=0.10.0"
}
},
"node_modules/end-of-stream": {
"version": "1.4.5",
"resolved": "https://registry.npmjs.org/end-of-stream/-/end-of-stream-1.4.5.tgz",