Compare commits
5 Commits
d45375a454
...
1c983fb144
| Author | SHA1 | Date | |
|---|---|---|---|
|
1c983fb144
|
|||
|
09fb6634f0
|
|||
|
46ddfdc464
|
|||
|
df6c3c50db
|
|||
|
8c5bed3e34
|
@@ -80,6 +80,19 @@ pub fn skill_file(name: &str) -> PathBuf {
|
|||||||
skill_dir(name).join("SKILL.md")
|
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 {
|
pub fn macros_dir() -> PathBuf {
|
||||||
match env::var(get_env_name("macros_dir")) {
|
match env::var(get_env_name("macros_dir")) {
|
||||||
Ok(value) => PathBuf::from(value),
|
Ok(value) => PathBuf::from(value),
|
||||||
@@ -286,3 +299,42 @@ pub fn local_models_override() -> Result<Vec<ProviderModels>> {
|
|||||||
}
|
}
|
||||||
Ok(models_override.list)
|
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:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -627,7 +627,15 @@ impl RequestContext {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
self.skill_registry.effective_role(&role)
|
match SkillPolicy::effective(
|
||||||
|
app,
|
||||||
|
self.role.as_ref(),
|
||||||
|
self.agent.as_ref(),
|
||||||
|
self.session.as_ref(),
|
||||||
|
) {
|
||||||
|
Ok(policy) => self.skill_registry.effective_role(&role, &policy),
|
||||||
|
Err(_) => role,
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn auto_continue_config(&self) -> AutoContinueConfig {
|
pub fn auto_continue_config(&self) -> AutoContinueConfig {
|
||||||
@@ -2576,6 +2584,7 @@ impl RequestContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn upsert_skill(&self, app: &AppConfig, name: &str) -> Result<()> {
|
pub fn upsert_skill(&self, app: &AppConfig, name: &str) -> Result<()> {
|
||||||
|
paths::validate_skill_name(name)?;
|
||||||
let path = paths::skill_file(name);
|
let path = paths::skill_file(name);
|
||||||
ensure_parent_exists(&path)?;
|
ensure_parent_exists(&path)?;
|
||||||
let is_new = !path.exists();
|
let is_new = !path.exists();
|
||||||
@@ -2594,6 +2603,7 @@ impl RequestContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub async fn load_skill_repl(&mut self, name: &str, abort_signal: AbortSignal) -> Result<()> {
|
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 {
|
if !self.app.config.function_calling_support {
|
||||||
bail!(
|
bail!(
|
||||||
"Skills require function calling, which is disabled. Enable function calling in your config then try again."
|
"Skills require function calling, which is disabled. Enable function calling in your config then try again."
|
||||||
|
|||||||
@@ -116,6 +116,7 @@ impl Skill {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn load(name: &str) -> Result<Self> {
|
pub fn load(name: &str) -> Result<Self> {
|
||||||
|
paths::validate_skill_name(name)?;
|
||||||
let path = paths::skill_file(name);
|
let path = paths::skill_file(name);
|
||||||
let content = read_to_string(&path)
|
let content = read_to_string(&path)
|
||||||
.with_context(|| format!("Failed to read skill '{name}' at {}", path.display()))?;
|
.with_context(|| format!("Failed to read skill '{name}' at {}", path.display()))?;
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
use super::role::{Role, RoleLike};
|
use super::role::{Role, RoleLike};
|
||||||
use super::skill::Skill;
|
use super::skill::Skill;
|
||||||
|
use super::skill_policy::SkillPolicy;
|
||||||
|
|
||||||
use anyhow::{Result, bail};
|
use anyhow::{Result, bail};
|
||||||
use indexmap::IndexMap;
|
use indexmap::IndexMap;
|
||||||
@@ -58,8 +59,8 @@ impl SkillRegistry {
|
|||||||
self.loaded.retain(|_, skill| !skill.auto_unload());
|
self.loaded.retain(|_, skill| !skill.auto_unload());
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn effective_role(&self, base: &Role) -> Role {
|
pub fn effective_role(&self, base: &Role, policy: &SkillPolicy) -> Role {
|
||||||
if self.loaded.is_empty() {
|
if !policy.skills_enabled || self.loaded.is_empty() {
|
||||||
return base.clone();
|
return base.clone();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -78,7 +79,10 @@ impl SkillRegistry {
|
|||||||
.map(|v| v.into_iter().collect())
|
.map(|v| v.into_iter().collect())
|
||||||
.unwrap_or_default();
|
.unwrap_or_default();
|
||||||
|
|
||||||
for (_, skill) in &self.loaded {
|
for (name, skill) in &self.loaded {
|
||||||
|
if !policy.allows(name) {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
if let Some(skill_tools) = skill.enabled_tools() {
|
if let Some(skill_tools) = skill.enabled_tools() {
|
||||||
tools.extend(skill_tools.iter().cloned());
|
tools.extend(skill_tools.iter().cloned());
|
||||||
}
|
}
|
||||||
@@ -113,6 +117,14 @@ impl SkillRegistry {
|
|||||||
fn insert_for_test(&mut self, skill: Skill) {
|
fn insert_for_test(&mut self, skill: Skill) {
|
||||||
self.loaded.insert(skill.name().to_string(), skill);
|
self.loaded.insert(skill.name().to_string(), skill);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn effective_role_for_test(&self, base: &Role) -> Role {
|
||||||
|
let policy = SkillPolicy {
|
||||||
|
skills_enabled: true,
|
||||||
|
enabled: self.loaded.keys().cloned().collect(),
|
||||||
|
};
|
||||||
|
self.effective_role(base, &policy)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
@@ -133,7 +145,7 @@ mod tests {
|
|||||||
let base = Role::new("test", "You are a helper");
|
let base = Role::new("test", "You are a helper");
|
||||||
let registry = SkillRegistry::default();
|
let registry = SkillRegistry::default();
|
||||||
|
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.prompt(), base.prompt());
|
assert_eq!(effective.prompt(), base.prompt());
|
||||||
}
|
}
|
||||||
@@ -144,7 +156,7 @@ mod tests {
|
|||||||
registry.insert_for_test(make_skill("git-master", "description: D", "Git knowledge"));
|
registry.insert_for_test(make_skill("git-master", "description: D", "Git knowledge"));
|
||||||
|
|
||||||
let base = Role::new("test", "You are a helper");
|
let base = Role::new("test", "You are a helper");
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.prompt(), "You are a helper\n\nGit knowledge");
|
assert_eq!(effective.prompt(), "You are a helper\n\nGit knowledge");
|
||||||
}
|
}
|
||||||
@@ -156,7 +168,7 @@ mod tests {
|
|||||||
registry.insert_for_test(make_skill("b", "", "Beta body"));
|
registry.insert_for_test(make_skill("b", "", "Beta body"));
|
||||||
|
|
||||||
let base = Role::new("test", "Base");
|
let base = Role::new("test", "Base");
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.prompt(), "Base\n\nAlpha body\n\nBeta body");
|
assert_eq!(effective.prompt(), "Base\n\nAlpha body\n\nBeta body");
|
||||||
}
|
}
|
||||||
@@ -168,7 +180,7 @@ mod tests {
|
|||||||
registry.insert_for_test(make_skill("b", "", "Beta"));
|
registry.insert_for_test(make_skill("b", "", "Beta"));
|
||||||
|
|
||||||
let base = Role::new("test", "");
|
let base = Role::new("test", "");
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.prompt(), "Alpha\n\nBeta");
|
assert_eq!(effective.prompt(), "Alpha\n\nBeta");
|
||||||
}
|
}
|
||||||
@@ -183,7 +195,7 @@ mod tests {
|
|||||||
));
|
));
|
||||||
|
|
||||||
let base = Role::new("test", "Process: __INPUT__");
|
let base = Role::new("test", "Process: __INPUT__");
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.prompt(), "Process: __INPUT__");
|
assert_eq!(effective.prompt(), "Process: __INPUT__");
|
||||||
let tools = effective.enabled_tools().expect("tools set by skill");
|
let tools = effective.enabled_tools().expect("tools set by skill");
|
||||||
@@ -196,7 +208,7 @@ mod tests {
|
|||||||
registry.insert_for_test(make_skill("knowledge", "enabled_tools: fs", ""));
|
registry.insert_for_test(make_skill("knowledge", "enabled_tools: fs", ""));
|
||||||
|
|
||||||
let base = Role::new("test", "Base");
|
let base = Role::new("test", "Base");
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.prompt(), "Base");
|
assert_eq!(effective.prompt(), "Base");
|
||||||
}
|
}
|
||||||
@@ -218,7 +230,7 @@ mod tests {
|
|||||||
let mut base = Role::new("test", "body");
|
let mut base = Role::new("test", "body");
|
||||||
base.set_enabled_tools(Some(vec!["web_search".to_string()]));
|
base.set_enabled_tools(Some(vec!["web_search".to_string()]));
|
||||||
|
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
let tools_vec = effective.enabled_tools().unwrap();
|
let tools_vec = effective.enabled_tools().unwrap();
|
||||||
let tools: BTreeSet<&str> = tools_vec.iter().map(|s| s.as_str()).collect();
|
let tools: BTreeSet<&str> = tools_vec.iter().map(|s| s.as_str()).collect();
|
||||||
@@ -235,7 +247,7 @@ mod tests {
|
|||||||
registry.insert_for_test(make_skill("knowledge", "", "Pure knowledge"));
|
registry.insert_for_test(make_skill("knowledge", "", "Pure knowledge"));
|
||||||
|
|
||||||
let base = Role::new("test", "Base");
|
let base = Role::new("test", "Base");
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert!(effective.enabled_tools().is_none());
|
assert!(effective.enabled_tools().is_none());
|
||||||
assert!(effective.enabled_mcp_servers().is_none());
|
assert!(effective.enabled_mcp_servers().is_none());
|
||||||
@@ -248,7 +260,7 @@ mod tests {
|
|||||||
|
|
||||||
let mut base = Role::new("test", "Base");
|
let mut base = Role::new("test", "Base");
|
||||||
base.set_enabled_tools(Some(Vec::new()));
|
base.set_enabled_tools(Some(Vec::new()));
|
||||||
let effective = registry.effective_role(&base);
|
let effective = registry.effective_role_for_test(&base);
|
||||||
|
|
||||||
assert_eq!(effective.enabled_tools().as_deref(), Some([].as_slice()));
|
assert_eq!(effective.enabled_tools().as_deref(), Some([].as_slice()));
|
||||||
}
|
}
|
||||||
|
|||||||
+10
-2
@@ -2,7 +2,7 @@ use super::state::StateManager;
|
|||||||
use super::structured;
|
use super::structured;
|
||||||
use super::types::LlmNode;
|
use super::types::LlmNode;
|
||||||
use crate::client::{Model, ModelType, call_chat_completions};
|
use crate::client::{Model, ModelType, call_chat_completions};
|
||||||
use crate::config::{Input, RequestContext, Role, RoleLike};
|
use crate::config::{Input, RequestContext, Role, RoleLike, SkillPolicy};
|
||||||
use crate::utils::create_abort_signal;
|
use crate::utils::create_abort_signal;
|
||||||
use anyhow::{Context, Error, Result, anyhow, bail};
|
use anyhow::{Context, Error, Result, anyhow, bail};
|
||||||
use serde_json::Value;
|
use serde_json::Value;
|
||||||
@@ -115,7 +115,15 @@ async fn run(
|
|||||||
|
|
||||||
let saved_agent_skill_state = swap_in_node_skill_policy(node, parent_ctx);
|
let saved_agent_skill_state = swap_in_node_skill_policy(node, parent_ctx);
|
||||||
|
|
||||||
let composed_role = parent_ctx.skill_registry.effective_role(&role);
|
let composed_role = match SkillPolicy::effective(
|
||||||
|
&parent_ctx.app.config,
|
||||||
|
parent_ctx.role.as_ref(),
|
||||||
|
parent_ctx.agent.as_ref(),
|
||||||
|
parent_ctx.session.as_ref(),
|
||||||
|
) {
|
||||||
|
Ok(policy) => parent_ctx.skill_registry.effective_role(&role, &policy),
|
||||||
|
Err(_) => role,
|
||||||
|
};
|
||||||
|
|
||||||
let saved_role = parent_ctx.role.clone();
|
let saved_role = parent_ctx.role.clone();
|
||||||
parent_ctx.role = Some(composed_role);
|
parent_ctx.role = Some(composed_role);
|
||||||
|
|||||||
+5
-4
@@ -16,6 +16,7 @@ use gman::providers::local::LocalProvider;
|
|||||||
use inquire::{Password, PasswordDisplayMode, required};
|
use inquire::{Password, PasswordDisplayMode, required};
|
||||||
use std::sync::{Arc, LazyLock};
|
use std::sync::{Arc, LazyLock};
|
||||||
use tokio::runtime::Handle;
|
use tokio::runtime::Handle;
|
||||||
|
use uuid::Uuid;
|
||||||
|
|
||||||
pub static SECRET_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{\{(.+)}}").unwrap());
|
pub static SECRET_RE: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"\{\{(.+)}}").unwrap());
|
||||||
|
|
||||||
@@ -175,22 +176,22 @@ impl Vault {
|
|||||||
}
|
}
|
||||||
|
|
||||||
pub fn validate_round_trip(&self) -> Result<()> {
|
pub fn validate_round_trip(&self) -> Result<()> {
|
||||||
const PROBE_KEY: &str = "__coyote_setup_probe__";
|
|
||||||
const PROBE_VALUE: &str = "ok";
|
const PROBE_VALUE: &str = "ok";
|
||||||
|
let probe_key = format!("__coyote_setup_probe_{}__", Uuid::new_v4().simple());
|
||||||
|
|
||||||
let h = Handle::current();
|
let h = Handle::current();
|
||||||
let result: Result<()> = tokio::task::block_in_place(|| {
|
let result: Result<()> = tokio::task::block_in_place(|| {
|
||||||
h.block_on(async {
|
h.block_on(async {
|
||||||
self.provider_ref()
|
self.provider_ref()
|
||||||
.set_secret(PROBE_KEY, PROBE_VALUE)
|
.set_secret(&probe_key, PROBE_VALUE)
|
||||||
.await
|
.await
|
||||||
.with_context(|| "vault write probe failed")?;
|
.with_context(|| "vault write probe failed")?;
|
||||||
let got = self
|
let got = self
|
||||||
.provider_ref()
|
.provider_ref()
|
||||||
.get_secret(PROBE_KEY)
|
.get_secret(&probe_key)
|
||||||
.await
|
.await
|
||||||
.with_context(|| "vault read probe failed")?;
|
.with_context(|| "vault read probe failed")?;
|
||||||
let _ = self.provider_ref().delete_secret(PROBE_KEY).await;
|
let _ = self.provider_ref().delete_secret(&probe_key).await;
|
||||||
if got != PROBE_VALUE {
|
if got != PROBE_VALUE {
|
||||||
bail!("vault read probe returned an unexpected value");
|
bail!("vault read probe returned an unexpected value");
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -364,6 +364,10 @@ pub fn interpolate_secrets(content: &str, vault: &Vault) -> Result<(String, Vec<
|
|||||||
|
|
||||||
SECRET_RE
|
SECRET_RE
|
||||||
.replace_all(line, |caps: &fancy_regex::Captures<'_>| {
|
.replace_all(line, |caps: &fancy_regex::Captures<'_>| {
|
||||||
|
if fatal_error.is_some() {
|
||||||
|
return String::new();
|
||||||
|
}
|
||||||
|
|
||||||
let name = caps[1].trim();
|
let name = caps[1].trim();
|
||||||
match vault.get_secret(name, false) {
|
match vault.get_secret(name, false) {
|
||||||
Ok(s) => s,
|
Ok(s) => s,
|
||||||
|
|||||||
Reference in New Issue
Block a user