Dark-Alex-17/coyote
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(TASK-048): install polish, hooks-dir mixin scan, script and probe hardening

Browse Source 
- one shared exec-bit predicate (Language-recognized extensions => 0755) used by
  every hook/agent installer; non-script files in hooks dirs stay non-executable
- mixin discovery scans the global hooks dir and each agent's hooks dir;
  .builtin-manifest is never scanned
- NTFS 8.3 short-name aliases rejected as builtin-manifest entries
- hook-script install announcements restricted to real hook locations
- COYOTE_HOOKS_DIR override warning compares canonicalized paths
- embedded assets decode via checked from_utf8 instead of from_utf8_unchecked
- log-events.sh defaults under ${XDG_STATE_HOME:-$HOME/.local/state}/coyote/hooks.log
  keeping 0600/symlink-refusal/secret-filter hardening; notify.sh tty fallback strips
  control bytes; both use UTC ISO-8601 timestamps
- payload files orphaned by a write failure are unlinked; stale coyote-hook-*.json
  payloads swept best-effort at startup
- sweep_removed_agent_hooks never follows symlinked agent dirs
- usage-probe-hooks.sh replaces bare sleeps with a bounded poll, re-verifies 0755 on
  restored scripts, and covers roles/agent hook locations
- TestConfigDirGuard hoisted into crate::testing
- .gitattributes pins LF line endings for bundled scripts

hooks is now a reserved bundle name.
This commit is contained in:
Dark-Alex-17 committed 2026-09-16 18:30:02 -06:00
1 parent e7bd75947c
commit 89a9398c28
16 files changed
+627 -353
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

src/config/agent.rs
+44 -46
View File
@@ -32,7 +32,7 @@ use inquire::{Text, validator::Validation};
use rust_embed::Embed;
use serde::{Deserialize, Serialize};
use std::collections::{BTreeMap, BTreeSet};
use std::{env, ffi::OsStr, path::Path};
use std::{env, path::Path};
const DEFAULT_AGENT_NAME: &str = "rag";
@@ -89,7 +89,8 @@ pub(crate) fn install_and_reconcile_agent_hooks(
}
ensure_parent_exists(&path)?;
info!("Creating agent hook file: {}", path.display());
write_file_atomic(&path, content, Some(0o755))?;
write_file_atomic(&path, content, None)?;
set_executable_bit_if_script(&path)?;
written.insert(name.clone());
}
@@ -127,6 +128,15 @@ fn sweep_removed_agent_hooks(bundled_files: &HashMap<String, HashSet<String>>) {
if bundled_files.contains_key(name) {
continue;
}
// A symlinked agent dir is user-arranged: never follow it into a
// hooks tree that lives somewhere else.
if !entry
.path()
.symlink_metadata()
.is_ok_and(|metadata| metadata.is_dir())
{
continue;
}
let hooks_dir = entry.path().join("hooks");
if !hooks_dir
.join(builtin_manifest::BUILTIN_MANIFEST_FILE)
@@ -184,14 +194,9 @@ impl Agent {
let embedded_file = AgentAssets::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded agent file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) };
let content = std::str::from_utf8(&embedded_file.data)
.expect("bundled agent asset is not valid UTF-8");
let file_path = paths::agents_data_dir().join(file.as_ref());
let file_extension = file_path
.extension()
.and_then(OsStr::to_str)
.map(|s| s.to_lowercase());
#[cfg_attr(not(unix), expect(unused))]
let is_script = matches!(file_extension.as_deref(), Some("sh") | Some("py"));
if file_path.exists()
&& !conflict::should_replace_existing(
@@ -213,18 +218,13 @@ impl Agent {
info!("Creating agent file: {}", file_path.display());
let mut agent_file = File::create(&file_path)?;
agent_file.write_all(content.as_bytes())?;
set_executable_bit_if_script(&file_path)?;
if let Some((agent, hook)) = parse_direct_hook_path(file.as_ref()) {
written_hooks
.entry(agent.to_string())
.or_default()
.insert(hook.to_string());
}
#[cfg(unix)]
if is_script {
use std::{fs, os::unix::fs::PermissionsExt};
fs::set_permissions(&file_path, fs::Permissions::from_mode(0o755))?;
}
}
let mut bundled_files: HashMap<String, HashSet<String>> = HashMap::new();
@@ -303,7 +303,8 @@ impl Agent {
};
let embedded = AgentAssets::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded agent file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded.data) };
let content = std::str::from_utf8(&embedded.data)
.expect("bundled agent hook asset is not valid UTF-8");
hooks.push((hook.to_string(), content.to_string()));
}
@@ -2058,36 +2059,7 @@ nodes: {}
assert_eq!(output, expected);
}
/// Points the config dir at a fresh temp directory for the guard's
/// lifetime and removes it on drop. Tests using it must serialize.
struct TestConfigDirGuard {
_env: crate::testing::EnvVarGuard,
root: PathBuf,
}
impl TestConfigDirGuard {
fn new(label: &str) -> Self {
let unique = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let root = env::temp_dir().join(format!("coyote-{label}-{unique}"));
std::fs::create_dir_all(&root).unwrap();
Self {
_env: crate::testing::EnvVarGuard::set(
crate::utils::get_env_name("config_dir"),
&root,
),
root,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.root);
}
}
use crate::testing::TestConfigDirGuard;
fn fixture(entries: &[(&str, &str)]) -> Vec<(String, String)> {
entries
@@ -2220,4 +2192,30 @@ nodes: {}
);
assert_eq!(read_to_string(dir.join("user.sh")).unwrap(), "user-owned");
}
#[cfg(unix)]
#[test]
#[serial_test::serial]
fn sweep_removed_agent_hooks_never_follows_symlinked_agent_dirs() {
let guard = TestConfigDirGuard::new("agent-sweep-symlink");
let real = guard.path.join("agent-elsewhere");
let hooks = real.join("hooks");
std::fs::create_dir_all(&hooks).unwrap();
std::fs::write(
hooks.join(builtin_manifest::BUILTIN_MANIFEST_FILE),
"shipped.sh\n",
)
.unwrap();
std::fs::write(hooks.join("shipped.sh"), "stale").unwrap();
std::fs::create_dir_all(paths::agents_data_dir()).unwrap();
std::os::unix::fs::symlink(&real, paths::agents_data_dir().join("linked")).unwrap();
sweep_removed_agent_hooks(&HashMap::new());
assert!(
hooks.join("shipped.sh").exists(),
"the sweep must never follow a symlinked agent dir"
);
assert!(hooks.join(builtin_manifest::BUILTIN_MANIFEST_FILE).exists());
}
}
src/config/builtin_manifest.rs
+46 -1
View File
@@ -23,6 +23,22 @@ pub(crate) fn is_builtin_manifest_name(name: &str) -> bool {
.eq_ignore_ascii_case(BUILTIN_MANIFEST_FILE)
}
/// NTFS 8.3 short names (`BUILTI~1`, `builti~1.sh`) can alias any long
/// filename on the same volume — including the manifest itself — so an entry
/// shaped like one is never accepted: it could direct a deletion at a file
/// the manifest never named.
fn is_ntfs_short_name_alias(name: &str) -> bool {
let stem = name.split('.').next().unwrap_or(name);
let Some((base, digits)) = stem.rsplit_once('~') else {
return false;
};
!base.is_empty()
&& base.len() <= 8
&& base.chars().all(|c| c.is_ascii_alphanumeric() || c == '~')
&& !digits.is_empty()
&& digits.chars().all(|c| c.is_ascii_digit())
}
/// Reconciles `dir` against the currently shipped set of builtin filenames:
/// a file is removed ONLY if it appears in the previous manifest AND is
/// absent from `shipped`. The manifest is then rewritten to the names this
@@ -83,7 +99,7 @@ fn read_manifest(dir: &Path) -> BTreeSet<String> {
if line.is_empty() || is_builtin_manifest_name(line) {
return false;
}
if !is_plain_file_name(line) {
if !is_plain_file_name(line) || is_ntfs_short_name_alias(line) {
debug!(
"Ignoring suspicious builtin manifest entry in {}: {line:?}",
path.display()
@@ -290,4 +306,33 @@ mod tests {
assert!(!is_builtin_manifest_name(".builtin-manifesto"));
assert!(!is_builtin_manifest_name("notify.sh"));
}
#[test]
fn ntfs_short_name_aliases_are_detected() {
assert!(is_ntfs_short_name_alias("BUILTI~1"));
assert!(is_ntfs_short_name_alias("builti~1.sh"));
assert!(is_ntfs_short_name_alias("NOTIFY~12"));
assert!(is_ntfs_short_name_alias("A~1"));
assert!(!is_ntfs_short_name_alias("notify.sh"));
assert!(!is_ntfs_short_name_alias("my~hook.sh"));
assert!(!is_ntfs_short_name_alias("~1"));
assert!(!is_ntfs_short_name_alias("way-too-long~1"));
assert!(!is_ntfs_short_name_alias(".builtin-manifest"));
}
#[test]
fn ntfs_short_name_entries_never_direct_deletions() {
let dir = fresh_dir("builtin-manifest-shortname-");
fs::write(dir.join(BUILTIN_MANIFEST_FILE), "BUILTI~1\nbuilti~1.sh\n").unwrap();
fs::write(dir.join("BUILTI~1"), "keep").unwrap();
fs::write(dir.join("builti~1.sh"), "keep").unwrap();
reconcile_builtin_dir(&dir, &BTreeSet::new(), &BTreeSet::new()).unwrap();
assert!(
dir.join("BUILTI~1").exists() && dir.join("builti~1.sh").exists(),
"8.3-alias-shaped manifest entries must never be removal candidates"
);
let _ = fs::remove_dir_all(&dir);
}
}
src/config/install_remote.rs
+30 -32
View File
@@ -4,9 +4,9 @@ use super::bundles::{
};
use crate::config::builtin_manifest::is_builtin_manifest_name;
use crate::config::conflict::{ConflictAction, NonInteractive, StickyMode, resolve_conflict};
use crate::config::{AssetCategory, BUNDLE_MANIFEST_FILE, InstallFilter, paths};
#[cfg(not(windows))]
use crate::function::Language;
use crate::config::{
AssetCategory, BUNDLE_MANIFEST_FILE, InstallFilter, paths, set_executable_bit_if_script,
};
use crate::mcp::{McpServer, McpServersConfig};
use crate::utils;
use crate::utils::IS_STDOUT_TERMINAL;
@@ -1858,18 +1858,18 @@ fn print_plan_summary(plan: &InstallPlan) {
}
/// Bundle-shipped hook scripts execute on the user's machine once wired into
/// config, so every planned file landing under a hooks/ directory is called
/// out individually. Visibility only: installing them needs no extra approval.
/// config, so every planned file landing in a hook location is called out
/// individually. Visibility only: installing them needs no extra approval.
///
/// Only the locations the engine actually resolves hooks from are announced:
/// the top-level hooks category, direct children of `agents/<name>/hooks/`,
/// and direct children of `roles/hooks/`. A `hooks/` directory anywhere else
/// (e.g. inside a skill or macro subtree) is inert data and announcing it
/// would be noise.
fn hook_script_lines(plan: &InstallPlan) -> Vec<String> {
plan.files
.iter()
.filter(|planned| {
planned.top_category == TopCategory::Hooks
|| planned
.rel
.parent()
.is_some_and(|parent| parent.components().any(|c| c.as_os_str() == "hooks"))
})
.filter(|planned| is_hook_location(planned))
.map(|planned| {
let rel = planned.rel.to_string_lossy().replace('\\', "/");
format!(
@@ -1880,6 +1880,16 @@ fn hook_script_lines(plan: &InstallPlan) -> Vec<String> {
.collect()
}
fn is_hook_location(planned: &PlannedFile) -> bool {
let components: Vec<_> = planned.rel.components().collect();
match planned.top_category {
TopCategory::Hooks => true,
TopCategory::Agents => components.len() == 3 && components[1].as_os_str() == "hooks",
TopCategory::Roles => components.len() == 2 && components[0].as_os_str() == "hooks",
_ => false,
}
}
fn count_kind(plan: &InstallPlan, cat: TopCategory, kind: PlannedKind) -> usize {
plan.files
.iter()
@@ -2042,26 +2052,6 @@ fn write_file(src: &Path, dst: &Path) -> Result<()> {
Ok(())
}
#[cfg(unix)]
fn set_executable_bit_if_script(path: &Path) -> Result<()> {
use std::os::unix::fs::PermissionsExt;
let Some(ext) = path.extension().and_then(OsStr::to_str) else {
return Ok(());
};
if Language::from_extension(ext) == Language::Unsupported {
return Ok(());
}
fs::set_permissions(path, fs::Permissions::from_mode(0o755))
.with_context(|| format!("chmod {}", path.display()))?;
Ok(())
}
#[cfg(not(unix))]
fn set_executable_bit_if_script(_path: &Path) -> Result<()> {
Ok(())
}
#[derive(Debug)]
struct McpMergeReport {
added: Vec<String>,
@@ -2874,6 +2864,14 @@ mod tests {
file("x/config.yaml", TopCategory::Agents),
file("reviewer.md", TopCategory::Roles),
file("hello.yaml", TopCategory::Macros),
// Only the locations the engine resolves hooks from announce:
// a hooks/ directory nested deeper or under another category
// is inert data.
file("x/hooks/nested/deep.sh", TopCategory::Agents),
file("x/sub/hooks/pre.sh", TopCategory::Agents),
file("sub/hooks/extra.sh", TopCategory::Roles),
file("my-skill/hooks/tool.sh", TopCategory::Skills),
file("my-macro/hooks/tool.sh", TopCategory::Macros),
],
mcp_json: None,
};
src/config/macros.rs
+9 -50
View File
@@ -167,7 +167,8 @@ impl Macro {
debug!("Processing macro file: {}", file.as_ref());
let embedded_file = MacroAssets::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded macro file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) };
let content = std::str::from_utf8(&embedded_file.data)
.expect("bundled macro asset is not valid UTF-8");
let file_path = paths::macros_dir().join(file.as_ref());
if file_path.exists() && !force {
@@ -350,56 +351,14 @@ fn default_true() -> bool {
mod tests {
use super::*;
use crate::config::{AppState, Session, WorkingMode};
use crate::testing::TestConfigDirGuard;
use crate::utils::{create_abort_signal, get_env_name};
use serial_test::serial;
use std::fs::{create_dir_all, remove_dir_all, write};
use std::future::Future;
use std::path::PathBuf;
use std::time::{SystemTime, UNIX_EPOCH};
use std::{env, str};
struct TestConfigDirGuard {
key: String,
previous: Option<std::ffi::OsString>,
path: PathBuf,
}
impl TestConfigDirGuard {
fn new() -> Self {
let key = get_env_name("config_dir");
let previous = env::var_os(&key);
let unique = SystemTime::now()
.duration_since(UNIX_EPOCH)
.unwrap()
.as_nanos();
let path = env::temp_dir().join(format!("coyote-macros-tests-{unique}"));
create_dir_all(&path).unwrap();
unsafe {
env::set_var(&key, &path);
}
Self {
key,
previous,
path,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
if let Some(previous) = &self.previous {
unsafe {
env::set_var(&self.key, previous);
}
} else {
unsafe {
env::remove_var(&self.key);
}
}
let _ = remove_dir_all(&self.path);
}
}
fn test_ctx() -> RequestContext {
RequestContext::new(Arc::new(AppState::test_default()), WorkingMode::Cmd)
}
@@ -974,7 +933,7 @@ variables:
#[test]
#[serial]
fn non_isolated_steps_run_on_live_ctx_and_mutations_persist() {
let _guard = TestConfigDirGuard::new();
let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file(
"live-macro",
"isolated: false\nsteps:\n - \".set temperature 0.42\"\n",
@@ -1006,7 +965,7 @@ variables:
#[test]
#[serial]
fn non_isolated_step_failure_aborts_and_restores_flag_and_mode() {
let _guard = TestConfigDirGuard::new();
let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file(
"fail-macro",
"isolated: false\nsteps:\n - \".set temperature 0.9\"\n - \".update\"\n - \".set temperature 0.1\"\n",
@@ -1057,7 +1016,7 @@ variables:
#[test]
#[serial]
fn non_isolated_macro_step_invoking_macro_is_rejected() {
let _guard = TestConfigDirGuard::new();
let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file(
"outer-macro",
"isolated: false\nsteps:\n - \".inner-macro\"\n",
@@ -1093,7 +1052,7 @@ variables:
#[test]
#[serial]
fn isolated_macro_step_runs_non_isolated_macro_inline_on_fork() {
let _guard = TestConfigDirGuard::new();
let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file("iso-outer-macro", "steps:\n - \".inner-macro\"\n");
write_macro_file(
"inner-macro",
@@ -1122,7 +1081,7 @@ variables:
#[test]
#[serial]
fn isolated_macro_still_forks_and_leaves_live_ctx_untouched() {
let _guard = TestConfigDirGuard::new();
let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file("iso-macro", "steps:\n - \".set temperature 0.77\"\n");
let mut ctx = test_ctx();
ctx.session = Some(Session::default());
@@ -1152,7 +1111,7 @@ variables:
#[test]
#[serial]
fn guard_restores_prior_flag_values_after_inline_run() {
let _guard = TestConfigDirGuard::new();
let _guard = TestConfigDirGuard::new("macros-tests");
write_macro_file(
"inner-macro",
"isolated: false\nsteps:\n - \".set temperature 0.11\"\n",
src/config/mod.rs
+31 -27
View File
@@ -419,6 +419,10 @@ impl Default for Config {
}
pub fn install_builtins() -> Result<()> {
// Best-effort housekeeping riding along with the startup installers:
// payload files orphaned by crashed processes accumulate in a shared
// temp dir and nothing else ever revisits them.
hooks::sweep_stale_payload_files();
Functions::install_builtin_global_tools(false)?;
Agent::install_builtin_agents(InstallMode::Skip)?;
Macro::install_macros(false)?;
@@ -1152,6 +1156,32 @@ pub(crate) fn ensure_parent_exists(path: &Path) -> Result<()> {
Ok(())
}
/// The one exec-bit policy for every hook/agent installer: a file is
/// executable iff `Language::from_extension` recognizes its extension.
/// Users may keep non-executable support files in hooks directories, so
/// installers must write plain and then apply this predicate — never
/// blanket-0755.
#[cfg(unix)]
pub(crate) fn set_executable_bit_if_script(path: &Path) -> Result<()> {
use crate::function::Language;
use std::os::unix::fs::PermissionsExt;
let Some(ext) = path.extension().and_then(std::ffi::OsStr::to_str) else {
return Ok(());
};
if Language::from_extension(ext) == Language::Unsupported {
return Ok(());
}
fs::set_permissions(path, fs::Permissions::from_mode(0o755))
.with_context(|| format!("chmod {}", path.display()))?;
Ok(())
}
#[cfg(not(unix))]
pub(crate) fn set_executable_bit_if_script(_path: &Path) -> Result<()> {
Ok(())
}
fn read_env_value<T>(key: &str) -> Option<Option<T>>
where
T: std::str::FromStr,
@@ -1824,33 +1854,7 @@ hooks:
}
}
/// Points the config dir at a fresh temp directory for the guard's
/// lifetime and removes it on drop. Tests using it must serialize.
struct TestConfigDirGuard {
_env: crate::testing::EnvVarGuard,
root: PathBuf,
}
impl TestConfigDirGuard {
fn new(label: &str) -> Self {
let unique = std::time::SystemTime::now()
.duration_since(std::time::UNIX_EPOCH)
.unwrap()
.as_nanos();
let root = env::temp_dir().join(format!("coyote-{label}-{unique}"));
fs::create_dir_all(&root).unwrap();
Self {
_env: crate::testing::EnvVarGuard::set(get_env_name("config_dir"), &root),
root,
}
}
}
impl Drop for TestConfigDirGuard {
fn drop(&mut self) {
let _ = fs::remove_dir_all(&self.root);
}
}
use crate::testing::TestConfigDirGuard;
#[test]
#[serial_test::serial]
src/config/role.rs
+41 -3
View File
@@ -180,7 +180,8 @@ impl Role {
pub fn builtin(name: &str) -> Result<Self> {
let content = RolesAsset::get(&format!("{name}.md"))
.ok_or_else(|| anyhow!("Unknown role `{name}`"))?;
let content = unsafe { std::str::from_utf8_unchecked(&content.data) };
let content =
std::str::from_utf8(&content.data).expect("bundled role asset is not valid UTF-8");
Ok(Role::new(name, content))
}
@@ -202,7 +203,8 @@ impl Role {
return None;
}
let embedded = RolesAsset::get(file.as_ref())?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded.data) };
let content = std::str::from_utf8(&embedded.data)
.expect("bundled role hook asset is not valid UTF-8");
Some((name.to_string(), content.to_string()))
})
.collect();
@@ -665,7 +667,8 @@ pub(crate) fn install_and_reconcile_role_hooks(
}
ensure_parent_exists(&path)?;
info!("Creating role hook file: {}", path.display());
write_file_atomic(&path, content, Some(0o755))?;
write_file_atomic(&path, content, None)?;
set_executable_bit_if_script(&path)?;
written.insert(name.clone());
}
@@ -1084,6 +1087,41 @@ Input 1
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
/// The installer writes plain and then applies the shared exec-bit
/// predicate: users may keep non-executable support files in hooks
/// directories, so only recognized script extensions get 0755.
#[cfg(unix)]
#[test]
fn role_hooks_install_leaves_non_script_files_non_executable() {
use std::os::unix::fs::PermissionsExt;
let dir = hooks_fixture_dir("role-hooks-nonscript-");
install_and_reconcile_role_hooks(
&dir,
&fixture(&[("README.md", "docs\n"), ("run.sh", "#!/bin/sh\n")]),
InstallMode::Skip,
&mut StickyMode::None,
)
.unwrap();
let readme_mode = fs::metadata(dir.join("README.md"))
.unwrap()
.permissions()
.mode();
assert_eq!(
readme_mode & 0o111,
0,
"a non-script file in a hooks dir must stay non-executable"
);
let script_mode = fs::metadata(dir.join("run.sh"))
.unwrap()
.permissions()
.mode();
assert_eq!(script_mode & 0o777, 0o755);
let _ = fs::remove_dir_all(dir.parent().unwrap());
}
#[test]
fn role_hooks_install_overwrites_only_with_force() {
let dir = hooks_fixture_dir("role-hooks-force-");
src/config/skill.rs
+2 -1
View File
@@ -100,7 +100,8 @@ impl Skill {
let embedded_file = SkillsAsset::get(&file)
.ok_or_else(|| anyhow!("Failed to load embedded skill file: {}", file.as_ref()))?;
let content = unsafe { std::str::from_utf8_unchecked(&embedded_file.data) };
let content = std::str::from_utf8(&embedded_file.data)
.expect("bundled skill asset is not valid UTF-8");
let file_path = paths::skills_dir().join(file.as_ref());
if file_path.exists() && !force {