From 4e317e243429d554fce0f435ab0616a2f1f9e50e Mon Sep 17 00:00:00 2001 From: Dorien Koelemeijer Date: Fri, 14 Nov 2025 08:02:58 +1000 Subject: [PATCH] first commit - wip - needs to be cleaned up --- .../src/developer/config_protection.rs | 135 ++++++++++++++++++ crates/goose-mcp/src/developer/mod.rs | 1 + .../goose-mcp/src/developer/rmcp_developer.rs | 12 ++ crates/goose-mcp/src/developer/text_editor.rs | 40 ++++++ 4 files changed, 188 insertions(+) create mode 100644 crates/goose-mcp/src/developer/config_protection.rs diff --git a/crates/goose-mcp/src/developer/config_protection.rs b/crates/goose-mcp/src/developer/config_protection.rs new file mode 100644 index 0000000000..f6c6287945 --- /dev/null +++ b/crates/goose-mcp/src/developer/config_protection.rs @@ -0,0 +1,135 @@ +//! Configuration file protection for developer tools +//! +//! This module prevents goose from modifying its own configuration files through +//! developer extension tools like text_editor and shell. + +use std::path::{Path, PathBuf}; + +/// Check if a path is within the goose configuration directory +pub fn is_goose_config_path(path: &Path) -> bool { + // Get the canonical config directory + let config_dir = match goose::config::paths::Paths::config_dir().canonicalize() { + Ok(dir) => dir, + Err(_) => return false, + }; + + // Try to canonicalize the target path + let canonical_path = match path.canonicalize() { + Ok(p) => p, + Err(_) => { + // If the file doesn't exist yet, check its parent directory + if let Some(parent) = path.parent() { + match parent.canonicalize() { + Ok(p) => p, + Err(_) => return false, + } + } else { + return false + } + } + }; + + // Check if the path is within the config directory + canonical_path.starts_with(&config_dir) +} + +/// Check if a shell command might modify goose configuration files +pub fn command_touches_config(command: &str) -> bool { + let config_dir = goose::config::paths::Paths::config_dir(); + let config_dir_str = config_dir.to_string_lossy(); + + // Check for common patterns that might modify config files + let dangerous_patterns = [ + "config.yaml", + "secrets.yaml", + ".config/goose", + &config_dir_str, + ]; + + let command_lower = command.to_lowercase(); + + // Check if command contains config-related paths + for pattern in &dangerous_patterns { + if command_lower.contains(&pattern.to_lowercase()) { + // Check if it's a write operation + if command_lower.contains('>') + || command_lower.contains("echo") + || command_lower.contains("cat") + || command_lower.contains("tee") + || command_lower.contains("sed") + || command_lower.contains("awk") + || command_lower.contains("rm") + || command_lower.contains("mv") + || command_lower.contains("cp") + || command_lower.contains("write") + || command_lower.contains("truncate") + { + return true; + } + } + } + + false +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + use tempfile::TempDir; + + #[test] + fn test_is_goose_config_path() { + let config_dir = goose::config::paths::Paths::config_dir(); + + // Test config.yaml + let config_yaml = config_dir.join("config.yaml"); + assert!(is_goose_config_path(&config_yaml)); + + // Test secrets.yaml + let secrets_yaml = config_dir.join("secrets.yaml"); + assert!(is_goose_config_path(&secrets_yaml)); + + // Test any file in config dir + let some_file = config_dir.join("some_file.txt"); + assert!(is_goose_config_path(&some_file)); + + // Test file outside config dir + let temp_dir = TempDir::new().unwrap(); + let outside_file = temp_dir.path().join("test.txt"); + fs::write(&outside_file, "test").unwrap(); + assert!(!is_goose_config_path(&outside_file)); + } + + #[test] + fn test_command_touches_config_write_operations() { + // Dangerous commands + assert!(command_touches_config("echo 'malicious' > ~/.config/goose/config.yaml")); + assert!(command_touches_config("cat data >> ~/.config/goose/config.yaml")); + assert!(command_touches_config("sed -i 's/old/new/' ~/.config/goose/config.yaml")); + assert!(command_touches_config("rm ~/.config/goose/config.yaml")); + assert!(command_touches_config("mv config.yaml ~/.config/goose/config.yaml")); + assert!(command_touches_config("cp backup.yaml ~/.config/goose/config.yaml")); + + // Safe commands (read-only) + assert!(!command_touches_config("cat ~/.config/goose/config.yaml")); + assert!(!command_touches_config("ls ~/.config/goose/")); + assert!(!command_touches_config("grep something ~/.config/goose/config.yaml")); + + // Commands with no config reference + assert!(!command_touches_config("echo 'hello' > /tmp/test.txt")); + assert!(!command_touches_config("ls -la")); + } + + #[test] + fn test_command_touches_config_case_insensitive() { + assert!(command_touches_config("ECHO 'test' > ~/.config/goose/CONFIG.YAML")); + assert!(command_touches_config("Echo 'test' > ~/.CONFIG/GOOSE/config.yaml")); + } + + #[test] + fn test_command_touches_config_secrets() { + assert!(command_touches_config("echo 'secret' > ~/.config/goose/secrets.yaml")); + assert!(command_touches_config("rm ~/.config/goose/secrets.yaml")); + } +} diff --git a/crates/goose-mcp/src/developer/mod.rs b/crates/goose-mcp/src/developer/mod.rs index 609f88bcc5..a7ad976b3e 100644 --- a/crates/goose-mcp/src/developer/mod.rs +++ b/crates/goose-mcp/src/developer/mod.rs @@ -1,4 +1,5 @@ pub mod analyze; +mod config_protection; mod editor_models; mod lang; mod shell; diff --git a/crates/goose-mcp/src/developer/rmcp_developer.rs b/crates/goose-mcp/src/developer/rmcp_developer.rs index e04132ab94..fba10616b6 100644 --- a/crates/goose-mcp/src/developer/rmcp_developer.rs +++ b/crates/goose-mcp/src/developer/rmcp_developer.rs @@ -28,6 +28,8 @@ use tokio::{ io::{AsyncBufReadExt, BufReader}, sync::RwLock, }; + +use super::config_protection; use tokio_stream::{wrappers::SplitStream, StreamExt as _}; use tokio_util::sync::CancellationToken; @@ -903,6 +905,16 @@ impl DeveloperServer { )); } + // Check if command attempts to modify goose configuration files + if config_protection::command_touches_config(command) { + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + "Access denied: Cannot execute commands that modify goose configuration files. \ + Configuration files can only be modified through the Settings UI or CLI.".to_string(), + None, + )); + } + let cmd_parts: Vec<&str> = command.split_whitespace().collect(); // Check if command arguments reference ignored files diff --git a/crates/goose-mcp/src/developer/text_editor.rs b/crates/goose-mcp/src/developer/text_editor.rs index d7f0bdb4a4..d16816c277 100644 --- a/crates/goose-mcp/src/developer/text_editor.rs +++ b/crates/goose-mcp/src/developer/text_editor.rs @@ -11,6 +11,7 @@ use url::Url; use rmcp::model::{Content, ErrorCode, ErrorData, Role}; +use super::config_protection; use super::editor_models::EditorModel; use super::lang; use super::shell::normalize_line_endings; @@ -702,6 +703,19 @@ pub async fn text_editor_view( } pub async fn text_editor_write(path: &PathBuf, file_text: &str) -> Result, ErrorData> { + // Protect goose configuration files from modification + if config_protection::is_goose_config_path(path) { + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "Access denied: Cannot modify goose configuration file '{}'. \ + Configuration files can only be modified through the Settings UI or CLI.", + path.display() + ), + None, + )); + } + // Normalize line endings based on platform let mut normalized_text = normalize_line_endings(file_text); // Make mutable @@ -755,6 +769,19 @@ pub async fn text_editor_replace( std::sync::Mutex>>, >, ) -> Result, ErrorData> { + // Protect goose configuration files from modification + if config_protection::is_goose_config_path(path) { + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "Access denied: Cannot modify goose configuration file '{}'. \ + Configuration files can only be modified through the Settings UI or CLI.", + path.display() + ), + None, + )); + } + // Check if diff is provided if let Some(diff_content) = diff { // Validate it's a proper diff @@ -924,6 +951,19 @@ pub async fn text_editor_insert( std::sync::Mutex>>, >, ) -> Result, ErrorData> { + // Protect goose configuration files from modification + if config_protection::is_goose_config_path(path) { + return Err(ErrorData::new( + ErrorCode::INVALID_PARAMS, + format!( + "Access denied: Cannot modify goose configuration file '{}'. \ + Configuration files can only be modified through the Settings UI or CLI.", + path.display() + ), + None, + )); + } + // Check if file exists if !path.exists() { return Err(ErrorData::new(