Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ux,v1.0] first pass at layering in trust checks #523

Open
wants to merge 6 commits into
base: v1.0
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crates/goose-server/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::Result;
use goose::providers::configs::GroqProviderConfig;
//use goose::trust_risk::TrustLevel;
use goose::{
agent::Agent,
developer::DeveloperSystem,
Expand All @@ -23,6 +24,7 @@ impl AppState {
let mut agent = Agent::new(provider);

dbg!("Adding DeveloperSystem");
//developer_system.set_trust_level(TrustLevel::NoDestructive);
agent.add_system(Box::new(DeveloperSystem::new()));

// Add NonDeveloperSystem only if GOOSE_SERVER__NON_DEVELOPER is set to "true"
Expand Down
34 changes: 34 additions & 0 deletions crates/goose/src/developer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ use xcap::{Monitor, Window};

use crate::errors::{AgentError, AgentResult};
use crate::systems::System;
use crate::trust_risk::{TrustLevel, TrustManager};
use mcp_core::{Content, Resource, Role, Tool, ToolCall};

pub struct DeveloperSystem {
Expand All @@ -24,6 +25,7 @@ pub struct DeveloperSystem {
active_resources: Mutex<HashMap<String, Resource>>, // Use URI string as key instead of PathBuf
file_history: Mutex<HashMap<PathBuf, Vec<String>>>,
instructions: String,
trust_manager: TrustManager,
}

impl Default for DeveloperSystem {
Expand All @@ -33,6 +35,16 @@ impl Default for DeveloperSystem {
}

impl DeveloperSystem {
/// Changes the trust level for operations
pub fn set_trust_level(&self, level: TrustLevel) {
self.trust_manager.set_level(level);
}

/// Gets the current trust level
pub fn get_trust_level(&self) -> TrustLevel {
self.trust_manager.get_level()
}

// Reads a resource from a URI and returns its content.
// The resource must already exist in active_resources.
pub async fn read_resource(&self, uri: &str) -> AgentResult<String> {
Expand Down Expand Up @@ -121,6 +133,7 @@ impl DeveloperSystem {
}

pub fn new() -> Self {
let trust_manager = TrustManager::new();
let list_windows_tool = Tool::new(
"list_windows",
indoc! {r#"
Expand Down Expand Up @@ -279,6 +292,7 @@ impl DeveloperSystem {
},
file_history: Mutex::new(HashMap::new()),
instructions,
trust_manager,
}
}

Expand Down Expand Up @@ -306,6 +320,15 @@ impl DeveloperSystem {
"The command string is required".into(),
))?;

// Only block destructive operations at level 0
if self.trust_manager.is_destructive_command(command)
&& self.trust_manager.get_level() == TrustLevel::NoDestructive
{
return Err(AgentError::ExecutionError(
"Do not run any destructive or editing commands. Can run commands but not if they make file changes or system changes.".into()
));
}

// Disallow commands that should use other tools
if command.trim_start().starts_with("cat") {
return Err(AgentError::InvalidParameters(
Expand Down Expand Up @@ -370,6 +393,15 @@ impl DeveloperSystem {
.and_then(|v| v.as_str())
.ok_or_else(|| AgentError::InvalidParameters("Missing 'path' parameter".into()))?;

// Only block destructive operations at level 0
if (command == "write" || command == "str_replace")
&& self.trust_manager.get_level() == TrustLevel::NoDestructive
{
return Err(AgentError::ExecutionError(
"File modifications are not allowed at NoDestructive trust level (0)".into(),
));
}

let path = self.resolve_path(path_str)?;

match command {
Expand Down Expand Up @@ -903,6 +935,8 @@ mod tests {
#[tokio::test]
async fn test_text_editor_write_and_view_file() {
let system = get_system().await;
// Set trust level to AllowAll for this test
system.set_trust_level(TrustLevel::AllowAll);

let temp_dir = tempfile::tempdir().unwrap();
let file_path = temp_dir.path().join("test.txt");
Expand Down
1 change: 1 addition & 0 deletions crates/goose/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,4 @@ pub mod prompt_template;
pub mod providers;
pub mod systems;
pub mod token_counter;
pub mod trust_risk;
179 changes: 179 additions & 0 deletions crates/goose/src/trust_risk.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
use std::sync::RwLock;

/// Represents different trust levels for performing operations
#[derive(Debug, Clone, Copy, PartialEq)]
pub enum TrustLevel {
/// No destructive actions allowed (0)
NoDestructive,
/// Confirm with user before destructive actions (1)
ConfirmDestructive,
/// Allow all actions without confirmation (2)
AllowAll,
}

impl Default for TrustLevel {
fn default() -> Self {
TrustLevel::ConfirmDestructive // Default to middle ground - confirm destructive actions
}
}

impl From<u8> for TrustLevel {
fn from(value: u8) -> Self {
match value {
0 => TrustLevel::NoDestructive,
1 => TrustLevel::ConfirmDestructive,
_ => TrustLevel::AllowAll,
}
}
}

impl From<TrustLevel> for u8 {
fn from(level: TrustLevel) -> Self {
match level {
TrustLevel::NoDestructive => 0,
TrustLevel::ConfirmDestructive => 1,
TrustLevel::AllowAll => 2,
}
}
}

/// Manages trust level state and provides methods to check and modify trust settings
pub struct TrustManager {
level: RwLock<TrustLevel>,
}

impl Default for TrustManager {
fn default() -> Self {
Self::new()
}
}

impl TrustManager {
/// Creates a new TrustManager with default trust level
pub fn new() -> Self {
Self {
level: RwLock::new(TrustLevel::default()),
}
}

/// Creates a new TrustManager with a specific trust level
pub fn with_level(level: TrustLevel) -> Self {
Self {
level: RwLock::new(level),
}
}

/// Gets the current trust level
pub fn get_level(&self) -> TrustLevel {
*self.level.read().unwrap()
}

/// Sets a new trust level
pub fn set_level(&self, new_level: TrustLevel) {
*self.level.write().unwrap() = new_level;
}

/// Checks if a destructive action is allowed
/// Returns true if the action should proceed, false if it should be blocked
pub fn can_perform_destructive(&self) -> bool {
match self.get_level() {
TrustLevel::NoDestructive => false,
TrustLevel::AllowAll => true,
TrustLevel::ConfirmDestructive => {
// In a real implementation, this would interact with the user
// For now, we'll just return false to be safe
false
}
}
}

/// Checks if a command is potentially destructive
/// This is a basic implementation that could be expanded
pub fn is_destructive_command(&self, command: &str) -> bool {
let command = command.trim().to_lowercase();

// List of destructive command prefixes
let destructive_prefixes = [
"rm",
"del",
"remove",
"write",
"overwrite",
"delete",
"drop",
];

destructive_prefixes
.iter()
.any(|prefix| command.starts_with(prefix))
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_trust_level_conversion() {
assert_eq!(TrustLevel::from(0), TrustLevel::NoDestructive);
assert_eq!(TrustLevel::from(1), TrustLevel::ConfirmDestructive);
assert_eq!(TrustLevel::from(2), TrustLevel::AllowAll);
assert_eq!(TrustLevel::from(255), TrustLevel::AllowAll); // Any value > 1 is AllowAll

assert_eq!(u8::from(TrustLevel::NoDestructive), 0);
assert_eq!(u8::from(TrustLevel::ConfirmDestructive), 1);
assert_eq!(u8::from(TrustLevel::AllowAll), 2);
}

#[test]
fn test_trust_manager_defaults() {
let manager = TrustManager::new();
assert_eq!(manager.get_level(), TrustLevel::ConfirmDestructive);
}

#[test]
fn test_trust_level_changes() {
let manager = TrustManager::new();

manager.set_level(TrustLevel::NoDestructive);
assert_eq!(manager.get_level(), TrustLevel::NoDestructive);

manager.set_level(TrustLevel::AllowAll);
assert_eq!(manager.get_level(), TrustLevel::AllowAll);
}

#[test]
fn test_destructive_command_detection() {
let manager = TrustManager::new();

// Test destructive commands
assert!(manager.is_destructive_command("rm -rf /"));
assert!(manager.is_destructive_command("delete file.txt"));
assert!(manager.is_destructive_command("remove old_data"));
assert!(manager.is_destructive_command("write new content"));

// Test non-destructive commands
assert!(!manager.is_destructive_command("ls"));
assert!(!manager.is_destructive_command("cd /"));
assert!(!manager.is_destructive_command("echo hello"));
assert!(!manager.is_destructive_command("pwd"));
}

#[test]
fn test_destructive_action_permissions() {
let manager = TrustManager::new();

// Test NoDestructive level
manager.set_level(TrustLevel::NoDestructive);
assert!(!manager.can_perform_destructive());

// Test AllowAll level
manager.set_level(TrustLevel::AllowAll);
assert!(manager.can_perform_destructive());

// Test ConfirmDestructive level
manager.set_level(TrustLevel::ConfirmDestructive);
// Currently returns false as user interaction is not implemented
assert!(!manager.can_perform_destructive());
}
}
Loading