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

fix(security): honor plugin enablement for skills (#11439)

Browse Source 
Signed-off-by: Jasper Hugo <jasper@spiral.xyz>
This commit is contained in:
Jasper
2026-08-21 15:36:31 +00:00
committed by GitHub
parent 290f508bfb
commit 45b322c1df
4 changed files with 1019 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/goose/src/plugins/discovery.rs
+139 -23
View File
@@ -1,4 +1,4 @@
use std::collections::HashMap;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use serde::{Deserialize, Serialize};
@@ -6,13 +6,13 @@ use serde::{Deserialize, Serialize};
use crate::config::{paths::Paths, Config};
use crate::plugins::plugin_install_dir;
const PLUGINS_CONFIG_KEY: &str = "plugins";
pub(in crate::plugins) const PLUGINS_CONFIG_KEY: &str = "plugins";
/// Per-plugin entry stored under the `plugins` map in `config.yaml`, keyed by
/// the plugin's filesystem path.
#[derive(Debug, Clone, Serialize, Deserialize)]
struct PluginConfigEntry {
enabled: bool,
pub(in crate::plugins) struct PluginConfigEntry {
pub enabled: bool,
}
/// A plugin found on disk and not disabled by any settings file.
@@ -53,36 +53,68 @@ pub fn discover_enabled_plugins(project_root: Option<&Path>) -> Vec<DiscoveredPl
discover_enabled_plugins_with_config(project_root, Config::global())
}
fn discover_enabled_plugins_with_config(
pub(crate) fn discover_enabled_plugins_with_config(
project_root: Option<&Path>,
config: &Config,
) -> Vec<DiscoveredPlugin> {
let scoped_settings = load_all_settings(project_root);
let mut found: HashMap<String, DiscoveredPlugin> = HashMap::new();
let user_plugins_dir = plugin_install_dir();
let mut found = Vec::new();
if let Some(root) = project_root {
for (name, root) in list_dir_children(&project_plugin_dir(root)) {
found.entry(name.clone()).or_insert(DiscoveredPlugin {
name,
root,
scope: PluginScope::Project,
});
let project_plugins_dir = project_plugin_dir(root);
if !equivalent_paths(&project_plugins_dir, &user_plugins_dir) {
found.extend(list_dir_children(&project_plugins_dir).into_iter().map(
|(name, root)| DiscoveredPlugin {
name,
root,
scope: PluginScope::Project,
},
));
}
}
for (name, root) in list_dir_children(&plugin_install_dir()) {
found.entry(name.clone()).or_insert(DiscoveredPlugin {
name,
root,
scope: PluginScope::User,
});
}
found.extend(
list_dir_children(&user_plugins_dir)
.into_iter()
.map(|(name, root)| DiscoveredPlugin {
name,
root,
scope: PluginScope::User,
}),
);
let enabled_by_settings: Vec<DiscoveredPlugin> = found
.into_values()
let mut enabled_plugins: Vec<DiscoveredPlugin> = filter_by_config(found, config)
.into_iter()
.filter(|plugin| is_enabled(&plugin.name, &scoped_settings))
.collect();
enabled_plugins.sort_by(|left, right| {
plugin_scope_rank(left.scope)
.cmp(&plugin_scope_rank(right.scope))
.then_with(|| left.name.cmp(&right.name))
.then_with(|| left.root.cmp(&right.root))
});
filter_by_config(enabled_by_settings, config)
let mut seen_names = HashSet::new();
enabled_plugins
.into_iter()
.filter(|plugin| seen_names.insert(plugin.name.clone()))
.collect()
}
fn equivalent_paths(left: &Path, right: &Path) -> bool {
left == right
|| left
.canonicalize()
.ok()
.zip(right.canonicalize().ok())
.is_some_and(|(left, right)| left == right)
}
fn plugin_scope_rank(scope: PluginScope) -> u8 {
match scope {
PluginScope::Project => 0,
PluginScope::User => 1,
}
}
/// Apply the `plugins` map in `config.yaml`. Newly discovered plugins are added
@@ -281,6 +313,7 @@ mod tests {
fn disabled_in_project_settings_drops_plugin() {
let tmp = tempfile::tempdir().unwrap();
let project = tmp.path();
let plugin_root = project.join(".agents/plugins/demo");
write_plugin_dir(&project.join(".agents").join("plugins"), "demo");
write_settings(
@@ -288,8 +321,16 @@ mod tests {
r#"{"disabledPlugins":["demo"]}"#,
);
let found = discover(project);
let cfg_dir = tempfile::tempdir().unwrap();
let config = test_config(cfg_dir.path());
let found = discover_with_config(project, &config);
assert!(found.iter().all(|p| p.name != "demo"));
let entries: HashMap<String, PluginConfigEntry> =
config.get_param(PLUGINS_CONFIG_KEY).unwrap();
assert!(entries
.get(&plugin_root.to_string_lossy().into_owned())
.is_some_and(|entry| entry.enabled));
}
#[test]
@@ -364,6 +405,48 @@ mod tests {
);
}
#[test]
fn disabled_project_plugin_falls_back_to_enabled_user_plugin_with_same_name() {
let project = tempfile::tempdir().unwrap();
let project_plugins = project.path().join(".agents/plugins");
write_plugin_dir(&project_plugins, "demo");
let path_root = tempfile::tempdir().unwrap();
let user_plugins = path_root.path().join(".agents/plugins");
write_plugin_dir(&user_plugins, "demo");
let config_dir = tempfile::tempdir().unwrap();
let config = test_config(config_dir.path());
config
.set_param(
PLUGINS_CONFIG_KEY,
HashMap::from([
(
project_plugins.join("demo").to_string_lossy().into_owned(),
PluginConfigEntry { enabled: false },
),
(
user_plugins.join("demo").to_string_lossy().into_owned(),
PluginConfigEntry { enabled: true },
),
]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let found = discover_enabled_plugins_with_config(Some(project.path()), &config);
let demo: Vec<_> = found
.into_iter()
.filter(|plugin| plugin.name == "demo")
.collect();
assert_eq!(demo.len(), 1);
assert_eq!(demo[0].scope, PluginScope::User);
assert_eq!(demo[0].root, user_plugins.join("demo"));
}
#[test]
fn newly_discovered_plugin_is_added_to_config_as_enabled() {
let tmp = tempfile::tempdir().unwrap();
@@ -439,4 +522,37 @@ mod tests {
config.get_param(PLUGINS_CONFIG_KEY).unwrap();
assert!(entries.get(&key).is_some_and(|e| e.enabled));
}
#[test]
fn orders_plugins_by_scope_then_name() {
let project = tempfile::tempdir().unwrap();
write_plugin_dir(&project.path().join(".agents/plugins"), "z-project-plugin");
write_plugin_dir(&project.path().join(".agents/plugins"), "a-project-plugin");
let path_root = tempfile::tempdir().unwrap();
write_plugin_dir(&path_root.path().join(".agents/plugins"), "z-user-plugin");
write_plugin_dir(&path_root.path().join(".agents/plugins"), "a-user-plugin");
let config_dir = tempfile::tempdir().unwrap();
let config = test_config(config_dir.path());
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let found = discover_enabled_plugins_with_config(Some(project.path()), &config);
let ordered: Vec<_> = found
.into_iter()
.map(|plugin| (plugin.name, plugin.scope))
.collect();
assert_eq!(
ordered,
vec![
("a-project-plugin".to_string(), PluginScope::Project),
("z-project-plugin".to_string(), PluginScope::Project),
("a-user-plugin".to_string(), PluginScope::User),
("z-user-plugin".to_string(), PluginScope::User),
]
);
}
}
crates/goose/src/plugins/mod.rs
+54 -18
View File
@@ -3,12 +3,14 @@ pub mod formats;
pub mod mcp_servers;
use crate::config::paths::Paths;
use crate::config::Config;
use crate::plugins::discovery::PluginScope;
use crate::subprocess::{git_command, SubprocessExt};
use anyhow::{anyhow, bail, Result};
use chrono::{DateTime, Duration, Utc};
use fs_err as fs;
use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use tracing::warn;
@@ -82,6 +84,49 @@ struct InstallMetadata {
}
pub fn installed_plugin_skill_dirs() -> Vec<PathBuf> {
enabled_plugin_skill_dirs(None)
.into_iter()
.map(|(path, _)| path)
.collect()
}
pub(crate) fn enabled_plugin_skill_dirs(
project_root: Option<&Path>,
) -> Vec<(PathBuf, PluginScope)> {
enabled_plugin_skill_dirs_with_config(project_root, Config::global())
}
fn is_project_plugin_install_dir(path: &Path) -> bool {
path.parent().is_some_and(|parent| {
parent.file_name().and_then(|name| name.to_str()) == Some("plugins")
&& parent
.parent()
.and_then(|grandparent| grandparent.file_name())
.and_then(|name| name.to_str())
== Some(".agents")
})
}
pub(crate) fn configured_project_plugin_skill_dirs(config: &Config) -> Vec<PathBuf> {
let entries: HashMap<String, discovery::PluginConfigEntry> = config
.get_param(discovery::PLUGINS_CONFIG_KEY)
.unwrap_or_default();
let user_plugins_dir = plugin_install_dir();
let mut seen = HashSet::new();
entries
.into_keys()
.map(PathBuf::from)
.filter(|path| is_project_plugin_install_dir(path) && !path.starts_with(&user_plugins_dir))
.flat_map(|path| formats::open_plugins::installed_skill_dirs(&path))
.filter(|path| seen.insert(path.clone()))
.collect()
}
pub(crate) fn enabled_plugin_skill_dirs_with_config(
project_root: Option<&Path>,
config: &Config,
) -> Vec<(PathBuf, PluginScope)> {
let plugins_dir = plugin_install_dir();
for update in auto_update_plugins_at_root(Utc::now(), &plugins_dir) {
if let Err(err) = update.result {
@@ -92,25 +137,16 @@ pub fn installed_plugin_skill_dirs() -> Vec<PathBuf> {
}
}
let entries = match fs::read_dir(plugins_dir) {
Ok(entries) => entries,
Err(_) => return Vec::new(),
};
let mut seen = HashSet::new();
entries
.flatten()
.flat_map(|entry| {
let plugin_dir = entry.path();
let default_skills_dir = plugin_dir.join("skills");
let mut skill_dirs = Vec::new();
if default_skills_dir.is_dir() {
skill_dirs.push(default_skills_dir);
}
skill_dirs.extend(formats::open_plugins::installed_skill_dirs(&plugin_dir));
skill_dirs
discovery::discover_enabled_plugins_with_config(project_root, config)
.into_iter()
.flat_map(|plugin| {
let plugin_dir = plugin.root;
formats::open_plugins::installed_skill_dirs(&plugin_dir)
.into_iter()
.map(move |dir| (dir, plugin.scope))
})
.filter(|path| seen.insert(path.clone()))
.filter(|(path, _)| seen.insert(path.clone()))
.collect()
}
crates/goose/src/skills/client.rs
+206 -6
View File
@@ -1,8 +1,9 @@
use super::discover_skills;
use super::discover_skills_with_config;
use super::loaded_skill_context_with_args;
use crate::agents::extension::PlatformExtensionContext;
use crate::agents::mcp_client::{Error, McpClientTrait};
use crate::agents::ToolCallContext;
use crate::config::Config;
use async_trait::async_trait;
use goose_sdk_types::custom_requests::{SourceEntry, SourceType};
use rmcp::model::{
@@ -19,6 +20,7 @@ pub struct SkillsClient {
info: InitializeResult,
working_dir: PathBuf,
exclude_builtin_skills: bool,
config: &'static Config,
}
impl SkillsClient {
@@ -36,6 +38,7 @@ impl SkillsClient {
info,
working_dir,
exclude_builtin_skills: false,
config: Config::global(),
})
}
@@ -46,8 +49,14 @@ impl SkillsClient {
self
}
#[cfg(test)]
fn with_config(mut self, config: &'static Config) -> Self {
self.config = config;
self
}
fn discover_skills(&self) -> Vec<SourceEntry> {
discover_skills(Some(&self.working_dir))
discover_skills_with_config(Some(&self.working_dir), self.config)
.into_iter()
.filter(|skill| {
!self.exclude_builtin_skills || skill.source_type != SourceType::BuiltinSkill
@@ -148,18 +157,28 @@ impl McpClientTrait for SkillsClient {
s.name == parent_skill_name
&& matches!(s.source_type, SourceType::Skill | SourceType::BuiltinSkill)
}) {
let skill_dir = PathBuf::from(&skill.path);
let listed_skill_dir = PathBuf::from(&skill.path);
let load_skill_dir = match listed_skill_dir.canonicalize() {
Ok(path) => path,
Err(e) => {
return Ok(CallToolResult::error(vec![ContentBlock::text(format!(
"Failed to resolve '{}': {}",
parent_skill_name, e
))]));
}
};
for file_path in &skill.supporting_files {
let file_path_buf = Path::new(file_path);
let Ok(rel) = file_path_buf.strip_prefix(&skill_dir) else {
let Ok(rel) = file_path_buf.strip_prefix(&listed_skill_dir) else {
continue;
};
if rel.to_string_lossy().replace('\\', "/") != relative_path {
continue;
}
let result = match super::load_supporting_file(&skill_dir, rel, skill_name) {
let result = match super::load_supporting_file(&load_skill_dir, rel, skill_name)
{
Ok(content) => CallToolResult::success(vec![ContentBlock::text(content)]),
Err(e) => CallToolResult::error(vec![ContentBlock::text(format!(
"Failed to read '{}': {}",
@@ -174,7 +193,7 @@ impl McpClientTrait for SkillsClient {
.iter()
.filter_map(|f| {
Path::new(f)
.strip_prefix(&skill_dir)
.strip_prefix(&listed_skill_dir)
.ok()
.map(|r| r.to_string_lossy().replace('\\', "/"))
})
@@ -256,10 +275,191 @@ impl McpClientTrait for SkillsClient {
#[cfg(test)]
mod tests {
use super::*;
use crate::config::Config;
use std::collections::HashMap;
use std::fs;
use std::sync::Arc;
use tempfile::TempDir;
fn write_plugin_skill(
project: &Path,
plugin_name: &str,
skill_name: &str,
description: &str,
body: &str,
) {
let skill_dir = project
.join(".agents/plugins")
.join(plugin_name)
.join("skills")
.join(skill_name);
fs::create_dir_all(&skill_dir).unwrap();
fs::write(
skill_dir.join("SKILL.md"),
format!("---\nname: {skill_name}\ndescription: {description}\n---\n{body}"),
)
.unwrap();
}
fn write_open_plugin_manifest(project: &Path, plugin_name: &str) {
let plugin_dir = project.join(".agents/plugins").join(plugin_name);
fs::write(
plugin_dir.join("plugin.json"),
format!(
r#"{{"name":"{plugin_name}","skills":{{"paths":["./skills","./custom-skills"]}}}}"#
),
)
.unwrap();
}
fn test_client(project: &Path, plugin_name: &str, enabled: bool) -> SkillsClient {
let config = Box::leak(Box::new(
Config::new(project.join("test-config.yaml"), "goose-skills-test").unwrap(),
));
let plugin_root = project.join(".agents/plugins").join(plugin_name);
config
.set_param(
"plugins",
HashMap::from([(
plugin_root.to_string_lossy().into_owned(),
HashMap::from([("enabled", enabled)]),
)]),
)
.unwrap();
let session = Arc::new(crate::session::Session {
working_dir: project.to_path_buf(),
..crate::session::Session::default()
});
SkillsClient::new(PlatformExtensionContext {
extension_manager: None,
session_manager: Arc::new(crate::session::SessionManager::instance()),
scheduler: None,
session: Some(session),
use_login_shell_path: false,
})
.unwrap()
.with_builtin_skills(false)
.with_config(config)
}
fn result_text(result: &CallToolResult) -> &str {
match &result.content[0] {
ContentBlock::Text(text) => &text.text,
_ => panic!("expected text"),
}
}
#[tokio::test]
async fn disabled_plugin_skill_is_not_listed_or_loadable() {
let _guard = env_lock::lock_env([("PLUGINS", None::<&str>)]);
let project = TempDir::new().unwrap();
write_plugin_skill(
project.path(),
"disabled-plugin",
"disabled-plugin-skill",
"Disabled plugin metadata",
"disabled plugin full body",
);
let client = test_client(project.path(), "disabled-plugin", false);
assert!(client
.get_instructions()
.is_none_or(|instructions| !instructions.contains("disabled-plugin-skill")));
let ctx = ToolCallContext::new("test".to_string(), None, None);
let args = serde_json::from_value(serde_json::json!({
"name": "disabled-plugin-skill"
}))
.unwrap();
let result = client
.call_tool(&ctx, "load_skill", Some(args), CancellationToken::new())
.await
.unwrap();
assert!(result.is_error.unwrap_or(false));
assert!(!result_text(&result).contains("disabled plugin full body"));
}
#[tokio::test]
async fn enabled_plugin_skill_is_listed_and_loadable() {
let _guard = env_lock::lock_env([("PLUGINS", None::<&str>)]);
let project = TempDir::new().unwrap();
write_plugin_skill(
project.path(),
"enabled-plugin",
"enabled-plugin-skill",
"Enabled plugin metadata",
"enabled plugin full body",
);
let custom_skill_dir = project
.path()
.join(".agents/plugins/enabled-plugin/custom-skills/custom-plugin-skill");
fs::create_dir_all(&custom_skill_dir).unwrap();
fs::write(
custom_skill_dir.join("SKILL.md"),
"---\nname: custom-plugin-skill\ndescription: Custom plugin metadata\n---\ncustom plugin full body",
)
.unwrap();
write_open_plugin_manifest(project.path(), "enabled-plugin");
let client = test_client(project.path(), "enabled-plugin", true);
let instructions = client.get_instructions().unwrap();
assert!(instructions.contains("enabled-plugin-skill"));
assert!(instructions.contains("Enabled plugin metadata"));
assert!(instructions.contains("custom-plugin-skill"));
assert!(instructions.contains("Custom plugin metadata"));
let ctx = ToolCallContext::new("test".to_string(), None, None);
let args = serde_json::from_value(serde_json::json!({
"name": "custom-plugin-skill"
}))
.unwrap();
let result = client
.call_tool(&ctx, "load_skill", Some(args), CancellationToken::new())
.await
.unwrap();
assert!(!result.is_error.unwrap_or(false));
assert!(result_text(&result).contains("custom plugin full body"));
}
#[cfg(unix)]
#[tokio::test]
async fn symlinked_project_plugin_supporting_file_is_loadable() {
let _guard = env_lock::lock_env([("PLUGINS", None::<&str>)]);
let project = TempDir::new().unwrap();
let external = TempDir::new().unwrap();
write_plugin_skill(
external.path(),
"symlinked-plugin",
"symlinked-skill",
"Symlinked skill metadata",
"symlinked skill body",
);
write_open_plugin_manifest(external.path(), "symlinked-plugin");
let external_plugin = external.path().join(".agents/plugins/symlinked-plugin");
let supporting_file = external_plugin.join("skills/symlinked-skill/guide.md");
fs::write(&supporting_file, "Symlinked supporting guidance.").unwrap();
let plugin_link = project.path().join(".agents/plugins/symlinked-plugin");
fs::create_dir_all(plugin_link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(&external_plugin, &plugin_link).unwrap();
let client = test_client(project.path(), "symlinked-plugin", true);
let ctx = ToolCallContext::new("test".to_string(), None, None);
let args = serde_json::from_value(serde_json::json!({
"name": "symlinked-skill/guide.md"
}))
.unwrap();
let result = client
.call_tool(&ctx, "load_skill", Some(args), CancellationToken::new())
.await
.unwrap();
assert!(!result.is_error.unwrap_or(false));
assert!(result_text(&result).contains("Symlinked supporting guidance."));
}
#[tokio::test]
async fn test_load_filesystem_skill_without_builtin_skills() {
let temp_dir = TempDir::new().unwrap();
crates/goose/src/skills/mod.rs
+620 -22
View File
@@ -11,7 +11,11 @@ pub use client::{SkillsClient, EXTENSION_NAME};
pub(crate) use supporting_files::{load_supporting_file, read_source_file};
use crate::config::{paths::Paths, Config};
use crate::plugins::installed_plugin_skill_dirs;
use crate::plugins::discovery::PluginScope;
use crate::plugins::{
configured_project_plugin_skill_dirs, enabled_plugin_skill_dirs_with_config,
installed_plugin_skill_dirs,
};
use crate::sources::parse_frontmatter;
use agent_client_protocol::Error;
use anyhow::Result;
@@ -192,6 +196,21 @@ fn canonicalize_or_original(path: &Path) -> PathBuf {
path.canonicalize().unwrap_or_else(|_| path.to_path_buf())
}
fn is_lexical_project_plugin_path(path: &Path) -> bool {
if path.starts_with(Paths::plugins_dir()) {
return false;
}
path.ancestors().any(|ancestor| {
ancestor.file_name().and_then(|name| name.to_str()) == Some("plugins")
&& ancestor
.parent()
.and_then(|parent| parent.file_name())
.and_then(|name| name.to_str())
== Some(".agents")
})
}
fn inferred_discoverable_skill_root(path: &Path) -> Option<PathBuf> {
let canonical_path = canonicalize_or_original(path);
@@ -225,16 +244,30 @@ fn inferred_discoverable_skill_root(path: &Path) -> Option<PathBuf> {
})
}
pub(crate) fn resolve_discoverable_skill_dir(path: &str) -> Result<PathBuf, Error> {
fn resolve_discoverable_skill_dir_with_config(
path: &str,
config: &Config,
) -> Result<PathBuf, Error> {
if path.is_empty() {
return Err(Error::invalid_params().data("Source path must not be empty"));
}
if is_lexical_project_plugin_path(Path::new(path)) {
return Err(Error::invalid_params().data(format!("Source \"{}\" not found", path)));
}
let canonical_dir = Path::new(path)
.canonicalize()
.map_err(|_| Error::invalid_params().data(format!("Source \"{}\" not found", path)))?;
if inferred_discoverable_skill_root(&canonical_dir).is_none()
let configured_project_plugin_path = !Path::new(path).starts_with(Paths::plugins_dir())
&& configured_project_plugin_skill_dirs(config)
.into_iter()
.map(|root| canonicalize_or_original(&root))
.any(|root| canonical_dir.starts_with(root));
if configured_project_plugin_path
|| inferred_discoverable_skill_root(&canonical_dir).is_none()
|| !canonical_dir.is_dir()
|| !canonical_dir.join("SKILL.md").is_file()
{
@@ -244,6 +277,10 @@ pub(crate) fn resolve_discoverable_skill_dir(path: &str) -> Result<PathBuf, Erro
Ok(canonical_dir)
}
pub(crate) fn resolve_discoverable_skill_dir(path: &str) -> Result<PathBuf, Error> {
resolve_discoverable_skill_dir_with_config(path, Config::global())
}
pub(crate) fn resolve_skill_dir(path: &str) -> Result<PathBuf, Error> {
resolve_discoverable_skill_dir(path)
}
@@ -316,34 +353,99 @@ pub(crate) fn parse_skill_frontmatter(raw: &str) -> (String, String) {
/// global (home-rooted) location. Order matches discovery precedence: project
/// dirs first, then global dirs.
pub fn all_skill_dirs(working_dir: Option<&Path>) -> Vec<(PathBuf, bool)> {
let mut dirs: Vec<(PathBuf, bool)> = Vec::new();
all_skill_dirs_with_config(working_dir, Config::global())
.into_iter()
.map(|dir| (dir.path, dir.is_global))
.collect()
}
struct SkillDirectory {
path: PathBuf,
is_global: bool,
writable: bool,
preserve_path: bool,
}
fn all_skill_dirs_with_config(working_dir: Option<&Path>, config: &Config) -> Vec<SkillDirectory> {
let mut dirs = Vec::new();
let plugin_dirs = enabled_plugin_skill_dirs_with_config(working_dir, config);
if let Some(wd) = working_dir {
dirs.push((wd.join(".agents").join("skills"), false));
dirs.push((wd.join(".goose").join("skills"), false));
dirs.push((wd.join(".claude").join("skills"), false));
for path in [
wd.join(".agents").join("skills"),
wd.join(".goose").join("skills"),
wd.join(".claude").join("skills"),
] {
dirs.push(SkillDirectory {
path,
is_global: false,
writable: true,
preserve_path: false,
});
}
}
dirs.extend(
plugin_dirs
.iter()
.filter(|(_, scope)| *scope == PluginScope::Project)
.map(|(path, _)| SkillDirectory {
path: path.clone(),
is_global: false,
writable: false,
preserve_path: true,
}),
);
let home = dirs::home_dir();
if let Some(h) = home.as_ref() {
dirs.push((h.join(".agents").join("skills"), true));
dirs.push(SkillDirectory {
path: h.join(".agents").join("skills"),
is_global: true,
writable: true,
preserve_path: false,
});
}
dirs.push((Paths::config_dir().join("skills"), true));
dirs.push(SkillDirectory {
path: Paths::config_dir().join("skills"),
is_global: true,
writable: true,
preserve_path: false,
});
if let Some(h) = home.as_ref() {
dirs.push((h.join(".claude").join("skills"), true));
dirs.push((h.join(".config").join("agents").join("skills"), true));
for path in [
h.join(".claude").join("skills"),
h.join(".config").join("agents").join("skills"),
] {
dirs.push(SkillDirectory {
path,
is_global: true,
writable: true,
preserve_path: false,
});
}
}
dirs.extend(
installed_plugin_skill_dirs()
plugin_dirs
.into_iter()
.map(|dir| (dir, true)),
.filter(|(_, scope)| *scope == PluginScope::User)
.map(|(path, _)| SkillDirectory {
path,
is_global: true,
writable: true,
preserve_path: true,
}),
);
dirs
}
fn parse_skill_content(content: &str, path: &Path, global: bool) -> Option<SourceEntry> {
fn parse_skill_content(
content: &str,
path: &Path,
global: bool,
writable: bool,
) -> Option<SourceEntry> {
let (metadata, body): (SkillFrontmatter, String) = match parse_frontmatter(content) {
Ok(Some(parsed)) => parsed,
Ok(None) => return None,
@@ -376,7 +478,7 @@ fn parse_skill_content(content: &str, path: &Path, global: bool) -> Option<Sourc
content: body,
path: path.to_string_lossy().into_owned(),
global,
writable: true,
writable,
supporting_files: Vec::new(),
properties: metadata.metadata,
})
@@ -424,7 +526,13 @@ fn walk_files_recursively<F, G>(
}
}
fn scan_skills_from_dir(dir: &Path, global: bool, seen: &mut HashSet<String>) -> Vec<SourceEntry> {
fn scan_skills_from_dir(
dir: &Path,
global: bool,
writable: bool,
preserve_path: bool,
seen: &mut HashSet<String>,
) -> Vec<SourceEntry> {
let mut skill_files = Vec::new();
let mut visited_dirs = HashSet::new();
@@ -444,8 +552,13 @@ fn scan_skills_from_dir(dir: &Path, global: bool, seen: &mut HashSet<String>) ->
let Some(skill_dir) = skill_file.parent() else {
continue;
};
let Ok(registered_skill_dir) = skill_dir.canonicalize() else {
continue;
let registered_skill_dir = if preserve_path {
skill_dir.to_path_buf()
} else {
let Ok(canonical_dir) = skill_dir.canonicalize() else {
continue;
};
canonical_dir
};
let content = match std::fs::read_to_string(&skill_file) {
Ok(c) => c,
@@ -455,7 +568,9 @@ fn scan_skills_from_dir(dir: &Path, global: bool, seen: &mut HashSet<String>) ->
}
};
if let Some(mut source) = parse_skill_content(&content, &registered_skill_dir, global) {
if let Some(mut source) =
parse_skill_content(&content, &registered_skill_dir, global, writable)
{
if !seen.contains(&source.name) {
let mut files = Vec::new();
let mut visited_support_dirs = HashSet::new();
@@ -490,17 +605,27 @@ fn scan_skills_from_dir(dir: &Path, global: bool, seen: &mut HashSet<String>) ->
/// Each returned entry has `global` set according to the directory it was
/// found in (or `true` for built-ins).
pub fn discover_skills(working_dir: Option<&Path>) -> Vec<SourceEntry> {
discover_skills_with_config(working_dir, Config::global())
}
fn discover_skills_with_config(working_dir: Option<&Path>, config: &Config) -> Vec<SourceEntry> {
let mut sources: Vec<SourceEntry> = Vec::new();
let mut seen = HashSet::new();
for (dir, is_global) in all_skill_dirs(working_dir) {
for source in scan_skills_from_dir(&dir, is_global, &mut seen) {
for dir in all_skill_dirs_with_config(working_dir, config) {
for source in scan_skills_from_dir(
&dir.path,
dir.is_global,
dir.writable,
dir.preserve_path,
&mut seen,
) {
sources.push(source);
}
}
for content in builtin::get_all() {
if let Some(source) = parse_skill_content(content, &PathBuf::new(), true) {
if let Some(source) = parse_skill_content(content, &PathBuf::new(), true, true) {
if !seen.contains(&source.name) {
seen.insert(source.name.clone());
let path = format!("builtin://skills/{}", source.name);
@@ -533,6 +658,44 @@ mod tests {
use super::*;
use serde_json::json;
fn write_test_skill(dir: &Path, name: &str, body: &str) {
std::fs::create_dir_all(dir).unwrap();
std::fs::write(
dir.join("SKILL.md"),
format!("---\nname: {name}\ndescription: Test skill\n---\n{body}"),
)
.unwrap();
}
fn assert_path_rejected_by_source_crud(path: &str, name: &str, skill_dir: &Path) {
let update_err = crate::sources::update_source_with_roots(
SourceType::Skill,
path,
name,
"updated",
"updated body",
crate::sources::UpdateSourceOptions {
properties: Some(HashMap::new()),
additional_roots: &[],
},
)
.unwrap_err();
assert!(format!("{update_err:?}").contains("not found"));
let delete_err = crate::sources::delete_source(SourceType::Skill, path).unwrap_err();
assert!(format!("{delete_err:?}").contains("not found"));
let export_err = crate::sources::export_source(SourceType::Skill, path).unwrap_err();
assert!(format!("{export_err:?}").contains("not found"));
assert!(skill_dir.join("SKILL.md").is_file());
}
fn assert_read_only_and_rejected_by_source_crud(skill: &SourceEntry, skill_dir: &Path) {
assert!(!skill.global);
assert!(!skill.writable);
assert_path_rejected_by_source_crud(&skill.path, &skill.name, skill_dir);
}
fn skill_with_content(content: &str) -> SourceEntry {
SourceEntry {
source_type: SourceType::Skill,
@@ -619,4 +782,439 @@ mod tests {
assert_eq!(rendered, skill.content);
}
#[test]
fn project_plugin_skill_precedes_global_skill_with_same_name() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let plugin_root = project.path().join(".agents/plugins/project-plugin");
write_test_skill(
&plugin_root.join("skills/collision"),
"collision",
"project plugin body",
);
write_test_skill(
&path_root.path().join("config/skills/collision"),
"collision",
"global body",
);
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_root.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skill = discover_skills_with_config(Some(project.path()), &config)
.into_iter()
.find(|skill| skill.name == "collision")
.unwrap();
assert_eq!(skill.content.trim(), "project plugin body");
assert!(!skill.global);
assert!(!skill.writable);
assert!(Path::new(&skill.path).starts_with(&plugin_root));
}
#[test]
fn user_plugin_skill_remains_writable_when_project_root_is_path_root() {
let path_root = tempfile::tempdir().unwrap();
let plugin_root = path_root.path().join(".agents/plugins/user-plugin");
write_test_skill(
&plugin_root.join("skills/user-owned"),
"user-owned",
"user body",
);
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_root.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skill = discover_skills_with_config(Some(path_root.path()), &config)
.into_iter()
.find(|skill| skill.name == "user-owned")
.unwrap();
assert!(skill.global);
assert!(skill.writable);
}
#[test]
fn exclusive_project_plugin_manifest_omits_default_skill_root() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let plugin_root = project.path().join(".agents/plugins/project-plugin");
write_test_skill(
&plugin_root.join("skills/excluded"),
"excluded",
"excluded body",
);
write_test_skill(
&plugin_root.join("custom-skills/included"),
"included",
"included body",
);
std::fs::write(
plugin_root.join("plugin.json"),
r#"{"name":"project-plugin","skills":{"exclusive":true,"paths":["./custom-skills"]}}"#,
)
.unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_root.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skills = discover_skills_with_config(Some(project.path()), &config);
assert!(skills.iter().any(|skill| skill.name == "included"));
assert!(!skills.iter().any(|skill| skill.name == "excluded"));
}
#[test]
fn project_plugin_skill_is_rejected_by_source_crud_before_discovery() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let plugin_root = project.path().join(".agents/plugins/project-plugin");
let skill_dir = plugin_root.join(".agents/skills/plugin-owned");
write_test_skill(&skill_dir, "plugin-owned", "plugin body");
std::fs::write(
plugin_root.join("plugin.json"),
r#"{"name":"project-plugin","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
assert_path_rejected_by_source_crud(
skill_dir.to_str().unwrap(),
"plugin-owned",
&skill_dir,
);
}
#[cfg(unix)]
#[test]
fn configured_disabled_project_plugin_canonical_path_is_rejected() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let external_root = tempfile::tempdir().unwrap();
let plugin_link = project.path().join(".agents/plugins/project-plugin");
let skill_dir = external_root.path().join(".agents/skills/plugin-owned");
write_test_skill(&skill_dir, "plugin-owned", "plugin body");
std::fs::write(
external_root.path().join("plugin.json"),
r#"{"name":"project-plugin","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
std::fs::create_dir_all(plugin_link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(external_root.path(), &plugin_link).unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_link.to_string_lossy().into_owned(),
HashMap::from([("enabled", false)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let err = resolve_discoverable_skill_dir_with_config(skill_dir.to_str().unwrap(), &config)
.unwrap_err();
assert!(format!("{err:?}").contains("not found"));
assert!(skill_dir.join("SKILL.md").is_file());
}
#[cfg(unix)]
#[test]
fn settings_disabled_project_plugin_canonical_path_is_rejected() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let external_root = tempfile::tempdir().unwrap();
let plugin_link = project.path().join(".agents/plugins/project-plugin");
let skill_dir = external_root.path().join(".agents/skills/plugin-owned");
write_test_skill(&skill_dir, "plugin-owned", "plugin body");
std::fs::write(
external_root.path().join("plugin.json"),
r#"{"name":"project-plugin","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
std::fs::create_dir_all(plugin_link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(external_root.path(), &plugin_link).unwrap();
let settings_dir = project.path().join(".config/goose");
std::fs::create_dir_all(&settings_dir).unwrap();
std::fs::write(
settings_dir.join("settings.json"),
r#"{"disabledPlugins":["project-plugin"]}"#,
)
.unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let plugins = crate::plugins::discovery::discover_enabled_plugins_with_config(
Some(project.path()),
&config,
);
assert!(plugins.iter().all(|plugin| plugin.name != "project-plugin"));
let err = resolve_discoverable_skill_dir_with_config(skill_dir.to_str().unwrap(), &config)
.unwrap_err();
assert!(format!("{err:?}").contains("not found"));
assert!(skill_dir.join("SKILL.md").is_file());
}
#[test]
fn nested_project_plugin_skill_is_listed_read_only_and_rejected_by_source_crud() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let plugin_root = project.path().join(".agents/plugins/project-plugin");
let skill_dir = plugin_root.join(".agents/skills/plugin-owned");
write_test_skill(&skill_dir, "plugin-owned", "plugin body");
std::fs::write(
plugin_root.join("plugin.json"),
r#"{"name":"project-plugin","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_root.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skill = discover_skills_with_config(Some(project.path()), &config)
.into_iter()
.find(|skill| skill.name == "plugin-owned")
.unwrap();
assert_read_only_and_rejected_by_source_crud(&skill, &skill_dir);
}
#[cfg(unix)]
#[test]
fn symlinked_project_plugin_skill_is_rejected_by_source_crud() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let external_root = tempfile::tempdir().unwrap();
let plugin_link = project.path().join(".agents/plugins/project-plugin");
let skill_dir = external_root.path().join(".agents/skills/plugin-owned");
write_test_skill(&skill_dir, "plugin-owned", "plugin body");
std::fs::write(
external_root.path().join("plugin.json"),
r#"{"name":"project-plugin","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
std::fs::create_dir_all(plugin_link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(external_root.path(), &plugin_link).unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_link.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skill = discover_skills_with_config(Some(project.path()), &config)
.into_iter()
.find(|skill| skill.name == "plugin-owned")
.unwrap();
assert!(Path::new(&skill.path).starts_with(&plugin_link));
assert_read_only_and_rejected_by_source_crud(&skill, &skill_dir);
}
#[cfg(unix)]
#[test]
fn symlinked_user_plugin_skill_remains_writable_for_source_crud() {
let path_root = tempfile::tempdir().unwrap();
let external_parent = tempfile::tempdir().unwrap();
let external_root = external_parent.path().join(".agents/plugins/user-plugin");
let skill_dir = external_root.join(".agents/skills/user-owned");
write_test_skill(&skill_dir, "user-owned", "user body");
std::fs::write(
external_root.join("plugin.json"),
r#"{"name":"user-plugin","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
let project = tempfile::tempdir().unwrap();
let project_path_root = tempfile::tempdir().unwrap();
let project_plugin_link = project.path().join(".agents/plugins/user-plugin");
std::fs::create_dir_all(project_plugin_link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(&external_root, &project_plugin_link).unwrap();
let project_config = Config::new(
project_path_root.path().join("test-config.yaml"),
"skills-test",
)
.unwrap();
project_config
.set_param(
"plugins",
HashMap::from([(
project_plugin_link.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
{
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", project_path_root.path().to_str()),
("PLUGINS", None),
]);
let project_skill = discover_skills_with_config(Some(project.path()), &project_config)
.into_iter()
.find(|skill| skill.name == "user-owned")
.unwrap();
assert!(!project_skill.writable);
}
let plugin_link = path_root.path().join(".agents/plugins/user-plugin");
std::fs::create_dir_all(plugin_link.parent().unwrap()).unwrap();
std::os::unix::fs::symlink(&external_root, &plugin_link).unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
config
.set_param(
"plugins",
HashMap::from([(
plugin_link.to_string_lossy().into_owned(),
HashMap::from([("enabled", true)]),
)]),
)
.unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skill = discover_skills_with_config(None, &config)
.into_iter()
.find(|skill| skill.name == "user-owned")
.unwrap();
assert!(skill.global);
assert!(skill.writable);
assert!(Path::new(&skill.path).starts_with(&plugin_link));
assert!(resolve_discoverable_skill_dir_with_config(&skill.path, &project_config).is_ok());
let updated = crate::sources::update_source_with_roots(
SourceType::Skill,
&skill.path,
"user-owned",
"updated",
"updated body",
crate::sources::UpdateSourceOptions {
properties: Some(HashMap::new()),
additional_roots: &[],
},
)
.unwrap();
assert_eq!(updated.content, "updated body");
let (exported, _) = crate::sources::export_source(SourceType::Skill, &skill.path).unwrap();
assert!(exported.contains("updated body"));
crate::sources::delete_source(SourceType::Skill, &skill.path).unwrap();
assert!(!skill_dir.exists());
}
#[test]
fn ordinary_project_skill_under_plugin_manifest_remains_writable() {
let project = tempfile::tempdir().unwrap();
let path_root = tempfile::tempdir().unwrap();
let skill_dir = project.path().join(".agents/skills/project-owned");
write_test_skill(&skill_dir, "project-owned", "project body");
std::fs::write(
project.path().join("plugin.json"),
r#"{"name":"ordinary-project","skills":{"paths":["./.agents/skills"]}}"#,
)
.unwrap();
let config = Config::new(path_root.path().join("test-config.yaml"), "skills-test").unwrap();
let _guard = env_lock::lock_env([
("GOOSE_PATH_ROOT", path_root.path().to_str()),
("PLUGINS", None),
]);
let skill = discover_skills_with_config(Some(project.path()), &config)
.into_iter()
.find(|skill| skill.name == "project-owned")
.unwrap();
assert!(skill.writable);
let updated = crate::sources::update_source_with_roots(
SourceType::Skill,
&skill.path,
"project-owned",
"updated",
"updated body",
crate::sources::UpdateSourceOptions {
properties: Some(HashMap::new()),
additional_roots: &[],
},
)
.unwrap();
assert_eq!(updated.content, "updated body");
let (exported, _) = crate::sources::export_source(SourceType::Skill, &skill.path).unwrap();
assert!(exported.contains("updated body"));
crate::sources::delete_source(SourceType::Skill, &skill.path).unwrap();
assert!(!skill_dir.exists());
}
}