fix: reject symlinked sub-agent files

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<SubAgent> {
+	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(&current) 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<bool> {
+	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<SubAgent> {
+	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<SubAgent> {
 ///
 /// 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");