From 38b14bbfc4ac2204557edb1b4715a0efdb84f9cf Mon Sep 17 00:00:00 2001 From: AkaraChen <85140972+AkaraChen@users.noreply.github.com> Date: Wed, 3 Jun 2026 17:20:42 +0800 Subject: [PATCH] fix: reject symlinked sub-agent files --- crates/agents/src/sub_agents.rs | 158 +++++++++++++++++++++++++++++++- 1 file changed, 155 insertions(+), 3 deletions(-) diff --git a/crates/agents/src/sub_agents.rs b/crates/agents/src/sub_agents.rs index fc6ec429..0fa3a74d 100644 --- a/crates/agents/src/sub_agents.rs +++ b/crates/agents/src/sub_agents.rs @@ -10,7 +10,9 @@ use crate::errors::{ConfigError, Result}; use crate::models::{ResourceScope, SubAgent}; use serde::{Deserialize, Serialize}; use std::fs; -use std::path::Path; +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::time::{SystemTime, UNIX_EPOCH}; // ── Frontmatter schema ─────────────────────────────────────────────────────── @@ -29,6 +31,9 @@ struct SubAgentFrontmatter { /// crate and uses the document body as the instruction. When the file has no /// frontmatter the file stem is used as the name. pub fn parse_sub_agent_file(path: &Path) -> Option { + if ensure_no_symlink_components(path).is_err() || !is_regular_file(path) { + return None; + } let content = fs::read_to_string(path).ok()?; let stem = path .file_stem() @@ -100,10 +105,117 @@ fn sanitize_filename(name: &str) -> String { out.trim_matches('-').to_string() } +fn is_regular_file(path: &Path) -> bool { + let Ok(meta) = fs::symlink_metadata(path) else { + return false; + }; + let file_type = meta.file_type(); + file_type.is_file() && !file_type.is_symlink() +} + +fn ensure_no_symlink_components(path: &Path) -> Result<()> { + let mut current = PathBuf::new(); + for component in path.components() { + current.push(component.as_os_str()); + let Ok(meta) = fs::symlink_metadata(¤t) else { + continue; + }; + if meta.file_type().is_symlink() { + return Err(ConfigError::InvalidConfig(format!( + "Refusing to use symlinked sub-agent path: {}", + current.display() + ))); + } + } + Ok(()) +} + +fn ensure_safe_sub_agent_dir(dir: &Path) -> Result<()> { + ensure_no_symlink_components(dir)?; + fs::create_dir_all(dir)?; + ensure_no_symlink_components(dir)?; + let meta = fs::symlink_metadata(dir)?; + let file_type = meta.file_type(); + if !file_type.is_dir() || file_type.is_symlink() { + return Err(ConfigError::InvalidConfig(format!( + "Sub-agent path is not a regular directory: {}", + dir.display() + ))); + } + Ok(()) +} + +fn assert_safe_destination(file: &Path) -> Result { + match fs::symlink_metadata(file) { + Ok(meta) => { + let file_type = meta.file_type(); + if !file_type.is_file() || file_type.is_symlink() { + return Err(ConfigError::InvalidConfig(format!( + "Refusing to overwrite unsafe sub-agent file: {}", + file.display() + ))); + } + Ok(true) + } + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(false), + Err(e) => Err(e.into()), + } +} + +fn unique_temp_path(dir: &Path, safe: &str) -> PathBuf { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or_default(); + dir.join(format!(".{safe}.{}.{}.tmp", std::process::id(), nanos)) +} + +fn write_sub_agent_file(file: &Path, content: &str) -> Result<()> { + let dir = file.parent().ok_or_else(|| { + ConfigError::InvalidConfig(format!( + "Sub-agent file has no parent directory: {}", + file.display() + )) + })?; + let existed = assert_safe_destination(file)?; + let safe = file + .file_stem() + .and_then(|n| n.to_str()) + .unwrap_or("sub-agent"); + let temp = unique_temp_path(dir, safe); + let mut handle = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&temp)?; + handle.write_all(content.as_bytes())?; + handle.sync_all()?; + drop(handle); + + if existed { + assert_safe_destination(file)?; + fs::remove_file(file)?; + } + if let Err(e) = fs::rename(&temp, file) { + let _ = fs::remove_file(&temp); + return Err(e.into()); + } + Ok(()) +} + // ── Directory-level I/O ────────────────────────────────────────────────────── /// Load sub-agents from a directory of `*.md` files. pub fn load_sub_agents_from_dir(dir: &Path) -> Vec { + if ensure_no_symlink_components(dir).is_err() { + return Vec::new(); + } + let Ok(meta) = fs::symlink_metadata(dir) else { + return Vec::new(); + }; + let file_type = meta.file_type(); + if !file_type.is_dir() || file_type.is_symlink() { + return Vec::new(); + } let Ok(entries) = fs::read_dir(dir) else { return Vec::new(); }; @@ -120,10 +232,10 @@ pub fn load_sub_agents_from_dir(dir: &Path) -> Vec { /// /// The directory is created if absent. pub fn save_sub_agent_to_dir(dir: &Path, agent: &SubAgent) -> Result<()> { - fs::create_dir_all(dir)?; + ensure_safe_sub_agent_dir(dir)?; let safe = sanitize_filename(&agent.name); let file = dir.join(format!("{safe}.md")); - fs::write(&file, format_sub_agent(agent)?)?; + write_sub_agent_file(&file, &format_sub_agent(agent)?)?; Ok(()) } @@ -243,6 +355,46 @@ mod tests { assert_eq!(loaded[0].instruction, Some("Do X.".to_string())); } + #[cfg(unix)] + #[test] + fn load_ignores_symlinked_markdown_files() { + use std::os::unix::fs::symlink; + + let dir = TempDir::new().unwrap(); + let target = dir.path().join("secret.md"); + let agents_dir = dir.path().join("agents"); + fs::create_dir(&agents_dir).unwrap(); + fs::write(&target, "TOP_SECRET").unwrap(); + symlink(&target, agents_dir.join("leak.md")).unwrap(); + + let loaded = load_sub_agents_from_dir(&agents_dir); + + assert!(loaded.is_empty()); + } + + #[cfg(unix)] + #[test] + fn save_refuses_to_overwrite_symlinked_markdown_files() { + use std::os::unix::fs::symlink; + + let dir = TempDir::new().unwrap(); + let target = dir.path().join("victim.md"); + fs::write(&target, "ORIGINAL").unwrap(); + symlink(&target, dir.path().join("evil.md")).unwrap(); + let agent = SubAgent { + name: "evil".to_string(), + description: None, + instruction: Some("OVERWRITE".to_string()), + source_path: None, + config_source: None, + }; + + let result = save_sub_agent_to_dir(dir.path(), &agent); + + assert!(result.is_err()); + assert_eq!(fs::read_to_string(&target).unwrap(), "ORIGINAL"); + } + #[test] fn sanitize_filename_basic() { assert_eq!(sanitize_filename("My Agent!"), "my-agent");