Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
3f55b3e
Pattern-matching only version of prompt injection detection - model s…
dorien-koelemeijer Aug 20, 2025
31e97c6
Add some more commands + try to include obfuscation
dorien-koelemeijer Aug 21, 2025
5402435
some cleanup after testing
dorien-koelemeijer Aug 21, 2025
5f0992f
Some further improvements in making sure threshold defined by user is…
dorien-koelemeijer Aug 21, 2025
374e71e
Try to add security scanning to the ToolMonitor and keep agent.rs cle…
dorien-koelemeijer Aug 22, 2025
4ade861
Clean up println
dorien-koelemeijer Aug 22, 2025
408939e
fix match on boolean
dorien-koelemeijer Aug 22, 2025
3208cfe
Address more comments - remove some unused code + add back AllowAlway…
dorien-koelemeijer Aug 25, 2025
d7ddebb
fix
dorien-koelemeijer Aug 25, 2025
b42fb60
fix: prevent flagged findings from resurfacing
dorien-koelemeijer Aug 25, 2025
1837c48
move permissions into tool monitor
dorien-koelemeijer Aug 26, 2025
3601aee
cleanup
dorien-koelemeijer Aug 27, 2025
dd74ad4
more cleanup
dorien-koelemeijer Aug 27, 2025
8b5d6cf
remove redundant else statement in security inspector
dorien-koelemeijer Aug 27, 2025
624e6d3
remove redundant test in tool_inspection
dorien-koelemeijer Aug 27, 2025
6df1201
get rid over provider in tool insepction manager
dorien-koelemeijer Aug 27, 2025
f9d25be
fix tool call confirmation pop up to prevent duplication of message
dorien-koelemeijer Aug 28, 2025
c32ac08
Rename inspector files + remove comment
dorien-koelemeijer Sep 1, 2025
17413d8
Update sorting of risks in patterns.rs
dorien-koelemeijer Sep 1, 2025
c1d775e
inspectors are in priority of how they get added to the tool monitor …
dorien-koelemeijer Sep 1, 2025
a0ba424
Update gooseMessage to take toolConfirmationContent object
dorien-koelemeijer Sep 1, 2025
c0f8711
small fix
dorien-koelemeijer Sep 1, 2025
719c900
Small styling update in tool call confirmation
dorien-koelemeijer Sep 1, 2025
d347eb1
remove configure_tool_monitor function
dorien-koelemeijer Sep 2, 2025
309831b
Fix code formatting
dorien-koelemeijer Sep 2, 2025
6ba3f2e
fix permissions issue - some cleanup to do be done still
dorien-koelemeijer Sep 2, 2025
381fe3c
another fix permission inspector issue - cleanup still to be done
dorien-koelemeijer Sep 2, 2025
516ac88
cleanup agent.rs after updating permission inspector
dorien-koelemeijer Sep 3, 2025
b8053e8
more cleanup + fix: add back always allow for non-security tool confi…
dorien-koelemeijer Sep 3, 2025
9fa9e71
Merge branch 'main' into feat/prompt-injection-pattern-matching-v1
dorien-koelemeijer Sep 4, 2025
589816e
Small fix after merge conflict resolution
dorien-koelemeijer Sep 4, 2025
480aa38
small fix permission manager
dorien-koelemeijer Sep 4, 2025
835459b
fix warnings - unused code
dorien-koelemeijer Sep 4, 2025
9a9955d
Merge branch 'block:main' into feat/prompt-injection-pattern-matching-v1
dorien-koelemeijer Sep 5, 2025
6714724
temp fix for issue caused by disabling of link previews
dorien-koelemeijer Sep 8, 2025
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
5 changes: 0 additions & 5 deletions crates/goose-cli/src/session/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,11 +256,6 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session {
process::exit(1);
});

// Configure tool monitoring if max_tool_repetitions is set
if let Some(max_repetitions) = session_config.max_tool_repetitions {
agent.configure_tool_monitor(Some(max_repetitions)).await;
}

// Handle session file resolution and resuming
let session_file: Option<std::path::PathBuf> = if session_config.no_session {
None
Expand Down
31 changes: 23 additions & 8 deletions crates/goose-cli/src/session/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -942,16 +942,31 @@ impl Session {
if let Some(MessageContent::ToolConfirmationRequest(confirmation)) = message.content.first() {
output::hide_thinking();

// Format the confirmation prompt
let prompt = "Goose would like to call the above tool, do you allow?".to_string();
// Format the confirmation prompt - use security message if present, otherwise use generic message
let prompt = if let Some(security_message) = &confirmation.prompt {
println!("\n{}", security_message);
"Do you allow this tool call?".to_string()
} else {
"Goose would like to call the above tool, do you allow?".to_string()
};

// Get confirmation from user
let permission_result = cliclack::select(prompt)
.item(Permission::AllowOnce, "Allow", "Allow the tool call once")
.item(Permission::AlwaysAllow, "Always Allow", "Always allow the tool call")
.item(Permission::DenyOnce, "Deny", "Deny the tool call")
.item(Permission::Cancel, "Cancel", "Cancel the AI response and tool call")
.interact();
let permission_result = if confirmation.prompt.is_none() {
// No security message - show all options including "Always Allow"
cliclack::select(prompt)
.item(Permission::AllowOnce, "Allow", "Allow the tool call once")
.item(Permission::AlwaysAllow, "Always Allow", "Always allow the tool call")
.item(Permission::DenyOnce, "Deny", "Deny the tool call")
.item(Permission::Cancel, "Cancel", "Cancel the AI response and tool call")
.interact()
} else {
// Security message present - don't show "Always Allow"
cliclack::select(prompt)
.item(Permission::AllowOnce, "Allow", "Allow the tool call once")
.item(Permission::DenyOnce, "Deny", "Deny the tool call")
.item(Permission::Cancel, "Cancel", "Cancel the AI response and tool call")
.interact()
};

let permission = match permission_result {
Ok(p) => p, // If Ok, use the selected permission
Expand Down
140 changes: 92 additions & 48 deletions crates/goose/src/agents/agent.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use std::collections::{HashMap, HashSet};
use std::collections::HashMap;
use std::future::Future;
use std::pin::Pin;
use std::sync::Arc;
Expand Down Expand Up @@ -31,18 +31,21 @@ use crate::agents::tool_route_manager::ToolRouteManager;
use crate::agents::tool_router_index_manager::ToolRouterIndexManager;
use crate::agents::types::SessionConfig;
use crate::agents::types::{FrontendTool, ToolResultReceiver};
use crate::config::{Config, ExtensionConfigManager, PermissionManager};
use crate::config::{Config, ExtensionConfigManager};
use crate::context_mgmt::auto_compact;
use crate::conversation::{debug_conversation_fix, fix_conversation, Conversation};
use crate::permission::permission_judge::{check_tool_permissions, PermissionCheckResult};
use crate::permission::permission_inspector::PermissionInspector;
use crate::permission::permission_judge::PermissionCheckResult;
use crate::permission::PermissionConfirmation;
use crate::providers::base::Provider;
use crate::providers::errors::ProviderError;
use crate::recipe::{Author, Recipe, Response, Settings, SubRecipe};
use crate::scheduler_trait::SchedulerTrait;
use crate::security::security_inspector::SecurityInspector;
use crate::session;
use crate::session::extension_data::ExtensionState;
use crate::tool_monitor::{ToolCall, ToolMonitor};
use crate::tool_inspection::ToolInspectionManager;
use crate::tool_monitor::RepetitionInspector;
use crate::utils::is_token_cancelled;
use mcp_core::ToolResult;
use regex::Regex;
Expand Down Expand Up @@ -81,8 +84,6 @@ pub struct ToolCategorizeResult {
pub frontend_requests: Vec<ToolRequest>,
pub remaining_requests: Vec<ToolRequest>,
pub filtered_response: Message,
pub readonly_tools: HashSet<String>,
pub regular_tools: HashSet<String>,
}

/// The main goose Agent
Expand All @@ -99,10 +100,11 @@ pub struct Agent {
pub(super) confirmation_rx: Mutex<mpsc::Receiver<(String, PermissionConfirmation)>>,
pub(super) tool_result_tx: mpsc::Sender<(String, ToolResult<Vec<Content>>)>,
pub(super) tool_result_rx: ToolResultReceiver,
pub(super) tool_monitor: Arc<Mutex<Option<ToolMonitor>>>,

pub(super) tool_route_manager: ToolRouteManager,
pub(super) scheduler_service: Mutex<Option<Arc<dyn SchedulerTrait>>>,
pub(super) retry_manager: RetryManager,
pub(super) tool_inspection_manager: ToolInspectionManager,
pub(super) autopilot: Mutex<AutoPilot>,
}

Expand Down Expand Up @@ -160,9 +162,6 @@ impl Agent {
let (confirm_tx, confirm_rx) = mpsc::channel(32);
let (tool_tx, tool_rx) = mpsc::channel(32);

let tool_monitor = Arc::new(Mutex::new(None));
let retry_manager = RetryManager::with_tool_monitor(tool_monitor.clone());

Self {
provider: Mutex::new(None),
extension_manager: ExtensionManager::new(),
Expand All @@ -176,17 +175,33 @@ impl Agent {
confirmation_rx: Mutex::new(confirm_rx),
tool_result_tx: tool_tx,
tool_result_rx: Arc::new(Mutex::new(tool_rx)),
tool_monitor,
tool_route_manager: ToolRouteManager::new(),
scheduler_service: Mutex::new(None),
retry_manager,
retry_manager: RetryManager::new(),
tool_inspection_manager: Self::create_default_tool_inspection_manager(),
autopilot: Mutex::new(AutoPilot::new()),
}
}

pub async fn configure_tool_monitor(&self, max_repetitions: Option<u32>) {
let mut tool_monitor = self.tool_monitor.lock().await;
*tool_monitor = Some(ToolMonitor::new(max_repetitions));
/// Create a tool inspection manager with default inspectors
fn create_default_tool_inspection_manager() -> ToolInspectionManager {
let mut tool_inspection_manager = ToolInspectionManager::new();

// Add security inspector (highest priority - runs first)
tool_inspection_manager.add_inspector(Box::new(SecurityInspector::new()));

// Add permission inspector (medium-high priority)
// Note: mode will be updated dynamically based on session config
tool_inspection_manager.add_inspector(Box::new(PermissionInspector::new(
"smart_approve".to_string(),
std::collections::HashSet::new(), // readonly tools - will be populated from extension manager
std::collections::HashSet::new(), // regular tools - will be populated from extension manager
)));

// Add repetition inspector (lower priority - basic repetition checking)
tool_inspection_manager.add_inspector(Box::new(RepetitionInspector::new(None)));

tool_inspection_manager
}

/// Reset the retry attempts counter to 0
Expand Down Expand Up @@ -247,6 +262,11 @@ impl Agent {
let (tools, toolshim_tools, system_prompt) = self.prepare_tools_and_prompt().await?;
let goose_mode = Self::determine_goose_mode(session.as_ref(), config);

// Update permission inspector mode to match the session mode
self.tool_inspection_manager
.update_permission_inspector_mode(goose_mode.clone())
.await;

Ok(ReplyContext {
messages: conversation,
tools,
Expand All @@ -261,10 +281,8 @@ impl Agent {
async fn categorize_tools(
&self,
response: &Message,
tools: &[rmcp::model::Tool],
_tools: &[rmcp::model::Tool],
) -> ToolCategorizeResult {
let (readonly_tools, regular_tools) = Self::categorize_tools_by_annotation(tools);

// Categorize tool requests
let (frontend_requests, remaining_requests, filtered_response) =
self.categorize_tool_requests(response).await;
Expand All @@ -273,8 +291,6 @@ impl Agent {
frontend_requests,
remaining_requests,
filtered_response,
readonly_tools,
regular_tools,
}
}

Expand Down Expand Up @@ -378,22 +394,6 @@ impl Agent {
cancellation_token: Option<CancellationToken>,
session: &Option<SessionConfig>,
) -> (String, Result<ToolCallResult, ErrorData>) {
// Check if this tool call should be allowed based on repetition monitoring
if let Some(monitor) = self.tool_monitor.lock().await.as_mut() {
let tool_call_info = ToolCall::new(tool_call.name.clone(), tool_call.arguments.clone());

if !monitor.check_tool_call(tool_call_info) {
return (
request_id,
Err(ErrorData::new(
ErrorCode::INTERNAL_ERROR,
"Tool call rejected: exceeded maximum allowed repetitions".to_string(),
None,
)),
);
}
}

if tool_call.name == PLATFORM_MANAGE_SCHEDULE_TOOL_NAME {
let result = self
.handle_schedule_management(tool_call.arguments, request_id.clone())
Expand Down Expand Up @@ -1119,8 +1119,6 @@ impl Agent {
frontend_requests,
remaining_requests,
filtered_response,
readonly_tools,
regular_tools,
} = self.categorize_tools(&response, &tools).await;
let requests_to_record: Vec<ToolRequest> = frontend_requests.iter().chain(remaining_requests.iter()).cloned().collect();
self.tool_route_manager
Expand Down Expand Up @@ -1159,16 +1157,40 @@ impl Agent {
);
}
} else {
let mut permission_manager = PermissionManager::default();
let (permission_check_result, enable_extension_request_ids) =
check_tool_permissions(
// Run all tool inspectors (security, repetition, permission, etc.)
let inspection_results = self.tool_inspection_manager
.inspect_tools(
&remaining_requests,
&mode,
readonly_tools.clone(),
regular_tools.clone(),
&mut permission_manager,
self.provider().await?,
).await;
messages.messages(),
)
.await?;

// Process inspection results into permission decisions using the permission inspector
let permission_check_result = self.tool_inspection_manager
.process_inspection_results_with_permission_inspector(
&remaining_requests,
&inspection_results,
)
.unwrap_or_else(|| {
// Fallback if permission inspector not found - default to needs approval
let mut result = PermissionCheckResult {
approved: vec![],
needs_approval: vec![],
denied: vec![],
};
result.needs_approval.extend(remaining_requests.iter().cloned());
result
});

// Track extension requests for special handling
let mut enable_extension_request_ids = vec![];
for request in &remaining_requests {
if let Ok(tool_call) = &request.tool_call {
if tool_call.name == PLATFORM_MANAGE_EXTENSIONS_TOOL_NAME {
enable_extension_request_ids.push(request.id.clone());
}
}
}

let mut tool_futures = self.handle_approved_and_denied_tools(
&permission_check_result,
Expand All @@ -1183,9 +1205,9 @@ impl Agent {
let mut tool_approval_stream = self.handle_approval_tool_requests(
&permission_check_result.needs_approval,
tool_futures_arc.clone(),
&mut permission_manager,
message_tool_response.clone(),
cancel_token.clone(),
&inspection_results,
);

while let Some(msg) = tool_approval_stream.try_next().await? {
Expand Down Expand Up @@ -1675,6 +1697,28 @@ mod tests {

assert!(todo_read.is_some(), "TODO read tool should be present");
assert!(todo_write.is_some(), "TODO write tool should be present");
Ok(())
}

#[tokio::test]
async fn test_tool_inspection_manager_has_all_inspectors() -> Result<()> {
let agent = Agent::new();

// Verify that the tool inspection manager has all expected inspectors
let inspector_names = agent.tool_inspection_manager.inspector_names();

assert!(
inspector_names.contains(&"repetition"),
"Tool inspection manager should contain repetition inspector"
);
assert!(
inspector_names.contains(&"permission"),
"Tool inspection manager should contain permission inspector"
);
assert!(
inspector_names.contains(&"security"),
"Tool inspection manager should contain security inspector"
);

Ok(())
}
Expand Down
23 changes: 0 additions & 23 deletions crates/goose/src/agents/reply_parts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use anyhow::Result;
use std::collections::HashSet;
use std::sync::Arc;

use async_stream::try_stream;
Expand Down Expand Up @@ -85,28 +84,6 @@ impl Agent {
Ok((tools, toolshim_tools, system_prompt))
}

/// Categorize tools based on their annotations
/// Returns:
/// - read_only_tools: Tools with read-only annotations
/// - non_read_tools: Tools without read-only annotations
pub(crate) fn categorize_tools_by_annotation(
tools: &[Tool],
) -> (HashSet<String>, HashSet<String>) {
tools
.iter()
.fold((HashSet::new(), HashSet::new()), |mut acc, tool| {
match &tool.annotations {
Some(annotations) if annotations.read_only_hint.unwrap_or(false) => {
acc.0.insert(tool.name.to_string());
}
_ => {
acc.1.insert(tool.name.to_string());
}
}
acc
})
}

/// Generate a response from the LLM provider
/// Handles toolshim transformations if needed
pub(crate) async fn generate_response_from_provider(
Expand Down
Loading
Loading