gitops/optimclaw
1
0
Fork 0
mirror of https://github.com/outbackdingo/optimclaw.git synced 2026-08-27 08:00:17 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat(signal) attachment upload + message tool (#375)

Browse Source 
* feat(channels/signal): add attachment upload support

- Add attachments field to OutgoingResponse for carrying file paths
- Add with_attachments() builder method to OutgoingResponse
- Update build_rpc_params() to include attachments array in JSON-RPC
- Update respond() and broadcast() to handle attachments:
  - Text + attachments: sends text first, then each attachment
  - Attachments only: sends each attachment with path as message
  - Text only: original behavior (no change)
- Add tests for build_rpc_params with attachments
- Add tests for OutgoingResponse attachment builder

This enables the Signal channel to send files via signal-cli daemon's
JSON-RPC send method, matching the nullclaw implementation.

Risk: Low - uses existing JSON-RPC infrastructure
Tests: 85 signal tests pass, 1543 lib tests pass

* feat(tools): add message tool for cross-channel messaging

Add a new 'message' tool that allows the agent to send messages to
any connected channel (signal, telegram, slack, etc.) with optional
file attachments.

Features:
- Send messages to specific channel + target combinations
- Support for attachments (file paths)
- E.164 validation delegated to channel (signal expects +number,
  telegram accepts username/chat_id, slack uses #channels)
- Helpful error messages showing available channels on failure

Tool schema:
- content: message text (required)
- channel: target channel name (optional, defaults to current channel)
- target: recipient (E.164, group ID, chat ID) (optional, defaults to
  current user/group chat)
- attachments: optional file paths to send

This complements the recently added attachment upload support for the
Signal channel by giving the agent a proper way to specify attachments
when sending messages.

Tests: 4 new tests for message tool schema
Risk: Low - new tool with no breaking changes
Tests: All 1547 lib tests pass, clippy clean

* feat(llm): add conversation context to system prompt for Signal

Add conversation_context HashMap to Reasoning struct to pass channel-specific
metadata (sender phone, sender UUID, group ID) to the LLM. This helps the
agent know who/group it's talking to, preventing it from hallucinating
phone numbers or sending to wrong recipients.

Changes:
- Add conversation_context field and with_conversation_data() builder method
- Add build_conversation_section() to include current conversation info in system prompt
- Update dispatcher to extract Signal metadata (sender, sender_uuid, group) and pass to Reasoning
- Add signal_sender_uuid to Signal channel metadata for privacy mode users

* feat(tools): add secure attachment path validation with sandbox enforcement

Implement robust path validation for message tool attachments to prevent
directory traversal attacks and unauthorized file access. Attachments are
now sandboxed to ~/.ironclaw/ by default.

Key changes:
- Create shared path_utils module with validate_path() and is_path_safe_basic()
- Extract normalize_lexical() from file.rs for reuse
- MessageTool now enforces sandbox at ~/.ironclaw/ for all attachments
- Path validation includes: traversal detection, canonicalization, symlink resolution
- Error messages reveal the allowed sandbox directory for user clarity

Security improvements:
- Blocks path traversal attacks (../, URL-encoded, null bytes)
- Canonicalizes paths to resolve symlinks before validation
- Walks up to nearest existing ancestor for non-existent paths
- Prevents escape from sandbox directory

Backward compatibility:
- File tools continue to work with their configured base_dir
- Message tool defaults to ~/.ironclaw/ sandbox
- Tests updated to create files within sandbox

Tests added:
- path_utils module tests (9 tests for validation logic)
- message tool attachment validation tests
- All 1571 existing tests pass

* fix(channels/signal): use robust path validation with full security coverage

Signal channel's validate_attachment_paths() now uses path_utils::validate_path()
for consistent, secure path validation.

Fixes:
- Replaced weak path.contains('..') check with robust validate_path()
- validate_path() now includes is_path_safe_basic() as first-pass filter to
  block null bytes and URL-encoded traversal sequences (%2e%2e%2f)
- Error message now shows allowed sandbox directory (~/.ironclaw/)

Security coverage:
- Path traversal: ../, foo/../bar, ../../etc/passwd ✓
- URL-encoded traversal: %2e%2e%2fetc/passwd ✓
- Null byte injection: file\0.txt ✓
- Paths outside sandbox: /tmp/evil.txt ✓
- Symlink escape attempts (via canonicalization) ✓

Tests added:
- validate_attachment_paths_rejects_path_outside_sandbox
- validate_attachment_paths_rejects_url_encoded_traversal
- validate_attachment_paths_rejects_null_byte
- Fixed broken assertion in rejects_double_dot test

* fix(llm): add Signal channel to build_channel_section to include message tool hint

The catch-all '_' arm was returning early before the message_tool_hint
section was constructed, which meant Signal users never got the
'## Proactive Messaging' section with examples for:
- Using attachments parameter
- Targeting different users/groups
- Cross-channel messaging

Now Signal will include the full message_tool_hint section with usage examples.

* fix(tools): use async locks in register_message_tools to prevent silent failures

The method was using register_sync which calls try_write() on self.tools.
If the lock was held, try_write() would return Err and silently skip
adding the tool to the registry, while self.message_tool already held
a reference. This creates an inconsistent state.

Fix: use async write locks directly instead of register_sync to ensure
the tool is always registered or the method fails explicitly.

* refactor(dispatcher): use Channel trait for conversation context

Replace hardcoded 'if message.channel == signal' block with generic
conversation_context() method on the Channel trait. This allows any
channel to provide context (sender, group, etc.) without hardcoding
channel names.

Changes:
- Add conversation_context() method to Channel trait (default: empty)
- Implement for SignalChannel: extracts sender, sender_uuid, group
- Add get_channel() to ChannelManager (returns Arc<dyn Channel>)
- Change ChannelManager storage from Box to Arc for shared access
- Update dispatcher to use new trait method
- Add tests for conversation_context extraction

Other channels (Telegram, Slack, Discord) can now implement this
method to provide conversation context without code changes in dispatcher.

* fix(tests): split message_tool_with_attachments into sandbox and channel tests

The original test was passing for the wrong reason - it expected an error
because the channel doesn't exist, but actually failed earlier during sandbox
validation because /tmp paths are outside ~/.ironclaw/.

Split into two tests:
- message_tool_with_attachments_outside_sandbox: verifies sandbox rejection
  with explicit error message check
- message_tool_with_attachments_inside_sandbox_no_channel: uses files within
  sandbox (like message_tool_passes_attachment_to_broadcast does) and verifies
  the channel-related error message

* security(message tool): add rate limiting, approval requirements, and audit logging

The message tool can send to ANY connected channel/target making it a significant
abuse vector if the LLM is compromised or prompt-injected. This commit adds:

1. Rate limiting: 10 messages/minute, 100/hour per user
2. Approval requirement: Always requires approval for cross-channel messages
   (when channel differs from the default conversation channel)
3. Audit logging: Every successful message send is logged with channel,
   target, and attachment count

The approval logic:
- If channel param is provided and differs from default -> Always require approval
- If no default channel is set and explicit channel provided -> Always require approval
- Otherwise (using default channel) -> UnlessAutoApproved

* fix(message tool): return explicit error for malformed attachments array

Previously, malformed attachments like {"attachments": [123, true]} would be
silently ignored via .ok().unwrap_or_default(), leaving users confused
when attachments weren't sent.

Now returns explicit error: "Invalid attachments format: ..."

* fix(message tool): verify attachment files exist before sending

Previously, non-existent paths would pass sandbox validation and surface
as confusing Signal RPC errors. Now returns clear "Attachment file not found" error.

* fix(test): create sandbox directory if it doesn't exist for CI

The test validate_attachment_paths_accepts_normal_paths uses
tempfile::tempdir_in() which requires the parent directory to exist.
In CI, ~/.ironclaw doesn't exist, causing test failure.
This commit is contained in:
ibhagwan
2026-02-26 18:06:21 +04:00
committed by GitHub
parent 1156884a49
commit bf35b59222
14 changed files with 1256 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/tools/builtin/file.rs
+2 -98
View File
@@ -11,6 +11,7 @@ use async_trait::async_trait;
use tokio::fs;
use crate::context::JobContext;
use crate::tools::builtin::path_utils::validate_path;
use crate::tools::tool::{
ApprovalRequirement, Tool, ToolDomain, ToolError, ToolOutput, require_str,
};
@@ -52,104 +53,6 @@ const MAX_WRITE_SIZE: usize = 5 * 1024 * 1024;
/// Maximum directory listing entries.
const MAX_DIR_ENTRIES: usize = 500;
/// Normalize a path by resolving `.` and `..` components lexically (no filesystem access).
///
/// This is critical for security: `std::fs::canonicalize` only works on paths that exist,
/// so for new files we must normalize without touching the filesystem.
fn normalize_lexical(path: &Path) -> PathBuf {
let mut components = Vec::new();
for component in path.components() {
match component {
std::path::Component::ParentDir => {
// Only pop if there's a normal component to pop (don't escape root/prefix)
if components
.last()
.is_some_and(|c| matches!(c, std::path::Component::Normal(_)))
{
components.pop();
}
}
std::path::Component::CurDir => {}
other => components.push(other),
}
}
components.iter().collect()
}
/// Validate that a path is safe (no traversal attacks).
///
/// For sandboxed paths (base_dir is set), we normalize the joined path lexically
/// and then verify it lives under the canonical base. This prevents escapes through
/// non-existent parent directories where `canonicalize()` would fall back to the
/// raw (un-normalized) path.
fn validate_path(path_str: &str, base_dir: Option<&Path>) -> Result<PathBuf, ToolError> {
let path = PathBuf::from(path_str);
// Resolve to absolute path
let resolved = if path.is_absolute() {
path.canonicalize()
.unwrap_or_else(|_| normalize_lexical(&path))
} else if let Some(base) = base_dir {
let joined = base.join(&path);
joined
.canonicalize()
.unwrap_or_else(|_| normalize_lexical(&joined))
} else {
let joined = std::env::current_dir()
.unwrap_or_else(|_| PathBuf::from("."))
.join(&path);
normalize_lexical(&joined)
};
// If base_dir is set, ensure the resolved path is within it
if let Some(base) = base_dir {
let base_canonical = base
.canonicalize()
.unwrap_or_else(|_| normalize_lexical(base));
// For existing paths, canonicalize to resolve symlinks.
// For non-existent paths, the lexical normalization above already removed
// all `..` components, so starts_with is reliable.
let check_path = if resolved.exists() {
resolved.canonicalize().unwrap_or_else(|_| resolved.clone())
} else {
// Walk up to the nearest existing ancestor directory, canonicalize it,
// then re-append the remaining tail. This handles the case where a
// symlink sits above the new file.
let mut ancestor = resolved.as_path();
let mut tail_parts: Vec<&std::ffi::OsStr> = Vec::new();
loop {
if ancestor.exists() {
let canonical_ancestor = ancestor
.canonicalize()
.unwrap_or_else(|_| ancestor.to_path_buf());
let mut result = canonical_ancestor;
for part in tail_parts.into_iter().rev() {
result = result.join(part);
}
break result;
}
if let Some(name) = ancestor.file_name() {
tail_parts.push(name);
}
match ancestor.parent() {
Some(parent) if parent != ancestor => ancestor = parent,
_ => break resolved.clone(),
}
}
};
if !check_path.starts_with(&base_canonical) {
return Err(ToolError::NotAuthorized(format!(
"Path escapes sandbox: {}",
path_str
)));
}
}
Ok(resolved)
}
/// Read file contents tool.
#[derive(Debug, Default)]
pub struct ReadFileTool {
@@ -723,6 +626,7 @@ impl Tool for ApplyPatchTool {
#[cfg(test)]
mod tests {
use super::*;
use crate::tools::builtin::path_utils::normalize_lexical;
use tempfile::TempDir;
#[tokio::test]
src/tools/builtin/message.rs
+519
View File
@@ -0,0 +1,519 @@
//! Message tool for sending messages to channels.
//!
//! Allows the agent to proactively message users on any connected channel.
use std::path::PathBuf;
use std::sync::Arc;
use async_trait::async_trait;
use tokio::sync::RwLock;
use crate::channels::{ChannelManager, OutgoingResponse};
use crate::context::JobContext;
use crate::tools::tool::{
ApprovalRequirement, Tool, ToolError, ToolOutput, ToolRateLimitConfig, require_str,
};
/// Tool for sending messages to channels.
pub struct MessageTool {
channel_manager: Arc<ChannelManager>,
/// Default channel for current conversation (set per-turn).
default_channel: Arc<RwLock<Option<String>>>,
/// Default target (user_id or group_id) for current conversation (set per-turn).
default_target: Arc<RwLock<Option<String>>>,
/// Base directory for attachment path validation (sandbox).
pub(crate) base_dir: PathBuf,
}
impl MessageTool {
pub fn new(channel_manager: Arc<ChannelManager>) -> Self {
let base_dir = dirs::home_dir()
.unwrap_or_else(|| std::path::PathBuf::from("."))
.join(".ironclaw");
Self {
channel_manager,
default_channel: Arc::new(RwLock::new(None)),
default_target: Arc::new(RwLock::new(None)),
base_dir,
}
}
/// Set the base directory for attachment validation.
/// This is primarily used for testing or future configuration.
pub fn with_base_dir(mut self, dir: PathBuf) -> Self {
self.base_dir = dir;
self
}
/// Set the default channel and target for the current conversation turn.
/// Call this before each agent turn with the incoming message's channel/target.
pub async fn set_context(&self, channel: Option<String>, target: Option<String>) {
*self.default_channel.write().await = channel;
*self.default_target.write().await = target;
}
}
#[async_trait]
impl Tool for MessageTool {
fn name(&self) -> &str {
"message"
}
fn description(&self) -> &str {
"Send a message to a channel. If channel/target omitted, uses the current conversation's \
channel and sender/group. Use to proactively message users on any connected channel. \
- Signal: target accepts E.164 (+1234567890) or group ID \
- Telegram: target accepts username or chat ID \
- Slack: target accepts channel (#general) or user ID"
}
fn parameters_schema(&self) -> serde_json::Value {
serde_json::json!({
"type": "object",
"properties": {
"content": {
"type": "string",
"description": "Message text to send"
},
"channel": {
"type": "string",
"description": "Target channel (defaults to current channel if omitted)"
},
"target": {
"type": "string",
"description": "Recipient: E.164 phone, group ID, chat ID (defaults to current sender/group if omitted)"
},
"attachments": {
"type": "array",
"items": {"type": "string"},
"description": "Optional file paths to attach to the message"
}
},
"required": ["content"]
})
}
async fn execute(
&self,
params: serde_json::Value,
_ctx: &JobContext,
) -> Result<ToolOutput, ToolError> {
let start = std::time::Instant::now();
let content = require_str(&params, "content")?;
// Get channel: use param or fall back to default
let channel = if let Some(c) = params.get("channel").and_then(|v| v.as_str()) {
c.to_string()
} else {
self.default_channel.read().await.clone().ok_or_else(|| {
ToolError::ExecutionFailed(
"No channel specified and no active conversation. Provide channel parameter."
.to_string(),
)
})?
};
// Get target: use param or fall back to default
let target = if let Some(t) = params.get("target").and_then(|v| v.as_str()) {
t.to_string()
} else {
self.default_target.read().await.clone().ok_or_else(|| {
ToolError::ExecutionFailed(
"No target specified and no active conversation. Provide target parameter."
.to_string(),
)
})?
};
let attachments: Vec<String> = match params.get("attachments") {
Some(v) => serde_json::from_value(v.clone()).map_err(|e| {
ToolError::ExecutionFailed(format!("Invalid attachments format: {}", e))
})?,
None => Vec::new(),
};
let attachment_count = attachments.len();
// Validate all attachment paths against the sandbox and verify existence
for path in &attachments {
let resolved =
crate::tools::builtin::path_utils::validate_path(path, Some(&self.base_dir))
.map_err(|e| {
ToolError::ExecutionFailed(format!(
"Attachment path must be within {}: {}",
self.base_dir.display(),
e
))
})?;
if !resolved.exists() {
return Err(ToolError::ExecutionFailed(format!(
"Attachment file not found: {}",
path
)));
}
}
let mut response = OutgoingResponse::text(content);
if !attachments.is_empty() {
response = response.with_attachments(attachments);
}
match self
.channel_manager
.broadcast(&channel, &target, response)
.await
{
Ok(()) => {
tracing::info!(
message_sent = true,
channel = %channel,
target = %target,
attachments = attachment_count,
"Message sent via message tool"
);
let msg = format!("Sent message to {}:{}", channel, target);
Ok(ToolOutput::text(msg, start.elapsed()))
}
Err(e) => {
let available = self.channel_manager.channel_names().await.join(", ");
let err_msg = if available.is_empty() {
format!(
"Failed to send to {}:{}: {}. No channels connected.",
channel, target, e
)
} else {
format!(
"Failed to send to {}:{}. Available channels: {}. Error: {}",
channel, target, available, e
)
};
Err(ToolError::ExecutionFailed(err_msg))
}
}
}
fn requires_approval(&self, params: &serde_json::Value) -> ApprovalRequirement {
// Require approval when sending to a different channel than the default
// (cross-channel messages are more sensitive)
let param_channel = params.get("channel").and_then(|v| v.as_str());
if let Some(channel) = param_channel {
// Check if it differs from the default channel
let default_channel = self.default_channel.blocking_read();
if let Some(default) = default_channel.as_ref()
&& channel != default
{
return ApprovalRequirement::Always;
}
// No default set - require approval for explicit channel selection
return ApprovalRequirement::Always;
}
// No channel specified in params - uses default, less risky
ApprovalRequirement::UnlessAutoApproved
}
fn rate_limit_config(&self) -> Option<ToolRateLimitConfig> {
Some(ToolRateLimitConfig::new(10, 100))
}
fn requires_sanitization(&self) -> bool {
false
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn message_tool_name() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
assert_eq!(tool.name(), "message");
}
#[test]
fn message_tool_description() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
assert!(!tool.description().is_empty());
}
#[test]
fn message_tool_schema_has_required_fields() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
let schema = tool.parameters_schema();
let params = schema.get("properties").unwrap();
assert!(params.get("content").is_some());
assert!(params.get("channel").is_some());
assert!(params.get("target").is_some());
// Only content is required - channel and target can be inferred from conversation context
let required = schema.get("required").unwrap().as_array().unwrap();
assert!(required.iter().any(|v| v == "content"));
assert!(!required.iter().any(|v| v == "channel"));
assert!(!required.iter().any(|v| v == "target"));
}
#[test]
fn message_tool_schema_has_optional_attachments() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
let schema = tool.parameters_schema();
let params = schema.get("properties").unwrap();
assert!(params.get("attachments").is_some());
}
#[tokio::test]
async fn message_tool_set_context_updates_defaults() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
// Initially no defaults set
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(serde_json::json!({"content": "hello"}), &ctx)
.await;
assert!(result.is_err()); // Should fail without defaults
// Set context
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Now execute should use the defaults (though it will fail because channel doesn't exist)
let result = tool
.execute(serde_json::json!({"content": "hello"}), &ctx)
.await;
// Will fail because channel doesn't exist, but should attempt to use the defaults
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(err.contains("signal") || err.contains("No channels connected"));
}
#[tokio::test]
async fn message_tool_explicit_params_override_defaults() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
// Set defaults
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Execute with explicit params - should fail but check that it uses explicit params
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"content": "hello",
"channel": "telegram",
"target": "@username"
}),
&ctx,
)
.await;
// Will fail because channel doesn't exist
assert!(result.is_err());
let err = result.unwrap_err().to_string();
// Should reference telegram, not signal
assert!(err.contains("telegram") || err.contains("No channels connected"));
}
#[tokio::test]
async fn message_tool_with_attachments_outside_sandbox() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
// Set context
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Execute with attachments outside sandbox
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"content": "hello",
"attachments": ["/tmp/file1.txt", "/tmp/file2.png"]
}),
&ctx,
)
.await;
// Should fail due to sandbox rejection (paths outside ~/.ironclaw/)
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(err.contains("sandbox") || err.contains("escapes"));
}
#[tokio::test]
async fn message_tool_with_attachments_inside_sandbox_no_channel() {
use std::fs;
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Create temp files inside the sandbox
let sandbox_dir = &tool.base_dir;
let temp_dir = tempfile::tempdir_in(sandbox_dir).unwrap();
let file1 = temp_dir.path().join("file1.txt");
let file2 = temp_dir.path().join("file2.png");
fs::write(&file1, "test").unwrap();
fs::write(&file2, "test").unwrap();
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"content": "hello",
"attachments": [file1.to_string_lossy(), file2.to_string_lossy()]
}),
&ctx,
)
.await;
// Path validation passes, but channel broadcast fails (no real channel)
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(err.contains("channel") || err.contains("Channel"));
}
#[tokio::test]
async fn message_tool_requires_content() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"channel": "signal",
"target": "+1234567890"
}),
&ctx,
)
.await;
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(err.contains("content") || err.contains("required"));
}
#[test]
fn message_tool_does_not_require_sanitization() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
assert!(!tool.requires_sanitization());
}
#[test]
fn path_traversal_rejects_double_dot() {
use crate::tools::builtin::path_utils::is_path_safe_basic;
assert!(!is_path_safe_basic("../etc/passwd"));
assert!(!is_path_safe_basic("foo/../bar"));
assert!(!is_path_safe_basic("foo/bar/../../secret"));
}
#[test]
fn path_traversal_accepts_normal_paths() {
use crate::tools::builtin::path_utils::is_path_safe_basic;
assert!(is_path_safe_basic("/tmp/file.txt"));
assert!(is_path_safe_basic("documents/report.pdf"));
assert!(is_path_safe_basic("my-file.png"));
}
#[tokio::test]
async fn message_tool_rejects_path_traversal_attachments() {
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"content": "here's the file",
"attachments": ["../../../etc/passwd"]
}),
&ctx,
)
.await;
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(err.contains("forbidden") || err.contains(".."));
}
#[tokio::test]
async fn message_tool_passes_attachment_to_broadcast() {
use std::fs;
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Create a temp file within the sandbox directory
let sandbox_dir = &tool.base_dir;
let temp_dir = tempfile::tempdir_in(sandbox_dir).unwrap();
let temp_path = temp_dir.path().join("test.txt");
fs::write(&temp_path, "test content").unwrap();
let temp_path_str = temp_path.to_string_lossy().to_string();
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"content": "here's the file",
"attachments": [temp_path_str]
}),
&ctx,
)
.await;
// Should succeed in path validation (file is in sandbox)
// but fail on channel broadcast (no actual channel)
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(
err.contains("not found") || err.contains("Failed") || err.contains("broadcast"),
"Expected channel error, got: {}",
err
);
}
#[tokio::test]
async fn message_tool_passes_multiple_attachments_to_broadcast() {
use std::fs;
let tool = MessageTool::new(Arc::new(ChannelManager::new()));
tool.set_context(Some("signal".to_string()), Some("+1234567890".to_string()))
.await;
// Create temp files within the sandbox directory
let sandbox_dir = &tool.base_dir;
let temp_dir = tempfile::tempdir_in(sandbox_dir).unwrap();
let temp_path1 = temp_dir.path().join("test1.txt");
let temp_path2 = temp_dir.path().join("test2.txt");
fs::write(&temp_path1, "test content 1").unwrap();
fs::write(&temp_path2, "test content 2").unwrap();
let path1 = temp_path1.to_string_lossy().to_string();
let path2 = temp_path2.to_string_lossy().to_string();
let ctx = crate::context::JobContext::new("test", "test description");
let result = tool
.execute(
serde_json::json!({
"content": "files attached",
"attachments": [path1, path2]
}),
&ctx,
)
.await;
// Should succeed in path validation (files are in sandbox)
// but fail on channel broadcast (no actual channel)
assert!(result.is_err());
let err = result.unwrap_err().to_string();
assert!(
err.contains("not found") || err.contains("Failed") || err.contains("broadcast"),
"Expected channel error, got: {}",
err
);
}
}
src/tools/builtin/mod.rs
+3
View File
@@ -7,6 +7,8 @@ mod http;
mod job;
mod json;
mod memory;
mod message;
pub mod path_utils;
pub mod routine;
pub(crate) mod shell;
pub mod skill_tools;
@@ -24,6 +26,7 @@ pub use job::{
};
pub use json::JsonTool;
pub use memory::{MemoryReadTool, MemorySearchTool, MemoryTreeTool, MemoryWriteTool};
pub use message::MessageTool;
pub use routine::{
RoutineCreateTool, RoutineDeleteTool, RoutineHistoryTool, RoutineListTool, RoutineUpdateTool,
};
src/tools/builtin/path_utils.rs
+239
View File
@@ -0,0 +1,239 @@
//! Shared path validation utilities for tools that access the filesystem.
//!
//! This module provides secure path validation to prevent directory traversal
//! attacks and ensure paths stay within allowed sandboxes.
use std::path::{Path, PathBuf};
use crate::tools::tool::ToolError;
/// Normalize a path by resolving `.` and `..` components lexically (no filesystem access).
///
/// This is critical for security: `std::fs::canonicalize` only works on paths that exist,
/// so for new files we must normalize without touching the filesystem.
pub fn normalize_lexical(path: &Path) -> PathBuf {
let mut components = Vec::new();
for component in path.components() {
match component {
std::path::Component::ParentDir => {
// Only pop if there's a normal component to pop (don't escape root/prefix)
if components
.last()
.is_some_and(|c| matches!(c, std::path::Component::Normal(_)))
{
components.pop();
}
}
std::path::Component::CurDir => {}
other => components.push(other),
}
}
components.iter().collect()
}
/// Validate that a path is safe (no traversal attacks).
///
/// For sandboxed paths (base_dir is set), we normalize the joined path lexically
/// and then verify it lives under the canonical base. This prevents escapes through
/// non-existent parent directories where `canonicalize()` would fall back to the
/// raw (un-normalized) path.
///
/// # Arguments
/// * `path_str` - The path to validate
/// * `base_dir` - Optional base directory for sandboxing
///
/// # Returns
/// * `Ok(resolved_path)` - The canonicalized, validated path
/// * `Err(ToolError)` - If path escapes sandbox or is invalid
pub fn validate_path(path_str: &str, base_dir: Option<&Path>) -> Result<PathBuf, ToolError> {
// First pass: reject null bytes and URL-encoded traversal
// Note: We don't block `..` here because validate_path handles it by
// normalizing lexically and checking sandbox containment
if !is_path_safe_minimal(path_str) {
return Err(ToolError::NotAuthorized(format!(
"Path contains forbidden characters or sequences: {}",
path_str
)));
}
let path = PathBuf::from(path_str);
// Resolve to absolute path
let resolved = if path.is_absolute() {
path.canonicalize()
.unwrap_or_else(|_| normalize_lexical(&path))
} else if let Some(base) = base_dir {
let joined = base.join(&path);
joined
.canonicalize()
.unwrap_or_else(|_| normalize_lexical(&joined))
} else {
let joined = std::env::current_dir()
.unwrap_or_else(|_| PathBuf::from("."))
.join(&path);
normalize_lexical(&joined)
};
// If base_dir is set, ensure the resolved path is within it
if let Some(base) = base_dir {
let base_canonical = base
.canonicalize()
.unwrap_or_else(|_| normalize_lexical(base));
// For existing paths, canonicalize to resolve symlinks.
// For non-existent paths, the lexical normalization above already removed
// all `..` components, so starts_with is reliable.
let check_path = if resolved.exists() {
resolved.canonicalize().unwrap_or_else(|_| resolved.clone())
} else {
// Walk up to the nearest existing ancestor directory, canonicalize it,
// then re-append the remaining tail. This handles the case where a
// symlink sits above the new file.
let mut ancestor = resolved.as_path();
let mut tail_parts: Vec<&std::ffi::OsStr> = Vec::new();
loop {
if ancestor.exists() {
let canonical_ancestor = ancestor
.canonicalize()
.unwrap_or_else(|_| ancestor.to_path_buf());
let mut result = canonical_ancestor;
for part in tail_parts.into_iter().rev() {
result = result.join(part);
}
break result;
}
if let Some(name) = ancestor.file_name() {
tail_parts.push(name);
}
match ancestor.parent() {
Some(parent) if parent != ancestor => ancestor = parent,
_ => break resolved.clone(),
}
}
};
if !check_path.starts_with(&base_canonical) {
return Err(ToolError::NotAuthorized(format!(
"Path escapes sandbox: {}",
path_str
)));
}
}
Ok(resolved)
}
/// Basic path safety check without requiring a base directory.
///
/// This is a fallback check that blocks obvious traversal attempts:
/// - Contains `..` components
/// - Contains null bytes
/// - Uses URL encoding to hide traversal
///
/// For stronger security, use validate_path() with a base_dir.
pub fn is_path_safe_basic(path: &str) -> bool {
// Block path traversal
if path.contains("..") {
return false;
}
// Block null bytes (would panic in Path)
if path.contains('\0') {
return false;
}
// Block URL-encoded traversal attempts
let lower = path.to_lowercase();
if lower.contains("%2e") || lower.contains("%2f") || lower.contains("%5c") {
return false;
}
true
}
/// Check for null bytes and URL-encoded traversal only.
/// Unlike is_path_safe_basic, this allows `..` in paths since validate_path
/// handles that by normalizing lexically and checking sandbox containment.
fn is_path_safe_minimal(path: &str) -> bool {
if path.contains('\0') {
return false;
}
let lower = path.to_lowercase();
if lower.contains("%2e") || lower.contains("%2f") || lower.contains("%5c") {
return false;
}
true
}
#[cfg(test)]
mod tests {
use super::*;
use tempfile::tempdir;
#[test]
fn test_is_path_safe_basic_allows_normal_paths() {
assert!(is_path_safe_basic("/tmp/file.txt"));
assert!(is_path_safe_basic("documents/report.pdf"));
assert!(is_path_safe_basic("my-file.png"));
}
#[test]
fn test_is_path_safe_basic_rejects_traversal() {
assert!(!is_path_safe_basic("../etc/passwd"));
assert!(!is_path_safe_basic("foo/../bar"));
assert!(!is_path_safe_basic("foo/bar/../../secret"));
}
#[test]
fn test_is_path_safe_basic_rejects_null_bytes() {
assert!(!is_path_safe_basic("file\0.txt"));
assert!(!is_path_safe_basic("/tmp/test\0.txt"));
}
#[test]
fn test_is_path_safe_basic_rejects_url_encoding() {
assert!(!is_path_safe_basic("%2e%2e%2fetc/passwd"));
assert!(!is_path_safe_basic("foo%2fbar"));
assert!(!is_path_safe_basic("test%5cpath"));
}
#[test]
fn test_validate_path_allows_within_sandbox() {
let dir = tempdir().unwrap();
let result = validate_path("subdir/file.txt", Some(dir.path()));
assert!(result.is_ok());
}
#[test]
fn test_validate_path_rejects_traversal_nonexistent_parent() {
let dir = tempdir().unwrap();
// Create a sibling directory structure to test escape
// Try to escape to parent and access /etc/passwd
let result = validate_path("../etc/passwd", Some(dir.path()));
assert!(result.is_err());
}
#[test]
fn test_validate_path_rejects_relative_traversal() {
let dir = tempdir().unwrap();
let result = validate_path("../../etc/passwd", Some(dir.path()));
assert!(result.is_err());
}
#[test]
fn test_validate_path_allows_valid_nested_write() {
let dir = tempdir().unwrap();
let result = validate_path("subdir/newfile.txt", Some(dir.path()));
assert!(result.is_ok());
}
#[test]
fn test_validate_path_allows_dot_dot_within_sandbox() {
let dir = tempdir().unwrap();
// This should be allowed as it stays within the sandbox
let result = validate_path("a/b/../c.txt", Some(dir.path()));
assert!(result.is_ok());
}
}
src/tools/registry.rs
+31
View File
@@ -67,6 +67,7 @@ const PROTECTED_TOOL_NAMES: &[&str] = &[
"skill_search",
"skill_install",
"skill_remove",
"message",
];
/// Registry of available tools.
@@ -80,6 +81,8 @@ pub struct ToolRegistry {
secrets_store: Option<Arc<dyn SecretsStore + Send + Sync>>,
/// Shared rate limiter for built-in tool invocations.
rate_limiter: RateLimiter,
/// Reference to the message tool for setting context per-turn.
message_tool: RwLock<Option<Arc<crate::tools::builtin::MessageTool>>>,
}
impl ToolRegistry {
@@ -91,6 +94,7 @@ impl ToolRegistry {
credential_registry: None,
secrets_store: None,
rate_limiter: RateLimiter::new(),
message_tool: RwLock::new(None),
}
}
@@ -399,6 +403,33 @@ impl ToolRegistry {
tracing::info!("Registered 5 routine management tools");
}
/// Register message tool for sending messages to channels.
pub async fn register_message_tools(
&self,
channel_manager: Arc<crate::channels::ChannelManager>,
) {
use crate::tools::builtin::MessageTool;
let tool = Arc::new(MessageTool::new(channel_manager));
*self.message_tool.write().await = Some(Arc::clone(&tool));
self.tools
.write()
.await
.insert(tool.name().to_string(), tool as Arc<dyn Tool>);
self.builtin_names
.write()
.await
.insert("message".to_string());
tracing::info!("Registered message tool");
}
/// Set the default channel and target for the message tool.
/// Call this before each agent turn with the current conversation's context.
pub async fn set_message_tool_context(&self, channel: Option<String>, target: Option<String>) {
if let Some(tool) = self.message_tool.read().await.as_ref() {
tool.set_context(channel, target).await;
}
}
/// Register the software builder tool.
///
/// The builder tool allows the agent to create new software including WASM tools,