Skip to content
Merged
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,5 @@ bench-results/
# WASM build artifacts (loaded from disk, not bundled)
*.wasm

# Traces
trace_*.json
2 changes: 1 addition & 1 deletion channels-src/slack/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion channels-src/slack/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "slack-channel"
version = "0.2.0"
version = "0.2.1"
edition = "2021"
description = "Slack Events API channel for IronClaw"
license = "MIT OR Apache-2.0"
Expand Down
104 changes: 104 additions & 0 deletions channels-src/slack/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,108 @@ fn extract_slack_attachments(files: &Option<Vec<SlackFile>>) -> Vec<InboundAttac
.collect()
}

/// Download a file from Slack using the url_private endpoint.
///
/// Slack file downloads require Bearer auth with the bot token, which is
/// injected by the host credential system via `channel_host::http_request`.
fn download_slack_file(url: &str) -> Result<Vec<u8>, String> {
let headers = serde_json::json!({});

let result = channel_host::http_request("GET", url, &headers.to_string(), None, None);

let response = result.map_err(|e| format!("Slack file download failed: {}", e))?;

if response.status != 200 {
let body_str = String::from_utf8_lossy(&response.body);
return Err(format!(
"Slack file download returned {}: {}",
response.status, body_str
));
}

Ok(response.body)
}

/// Download file bytes and store them via the host for processing.
///
/// Downloads all file types (images, documents, etc.) so the host-side
/// middleware can process them (vision pipeline for images, text extraction
/// for documents, transcription for audio, etc.).
/// Maximum file size to download (20 MB). Files larger than this are skipped
/// to avoid excessive memory use and slow downloads in the WASM runtime.
const MAX_DOWNLOAD_SIZE_BYTES: u64 = 20 * 1024 * 1024;

fn download_and_store_slack_files(attachments: &[InboundAttachment]) {
for att in attachments {
let Some(ref url) = att.source_url else {
continue;
};

// Skip files that exceed the size limit
if let Some(size) = att.size_bytes {
if size > MAX_DOWNLOAD_SIZE_BYTES {
channel_host::log(
channel_host::LogLevel::Warn,
&format!(
"Skipping Slack file download: {} bytes exceeds {} MB limit (id={})",
size,
MAX_DOWNLOAD_SIZE_BYTES / (1024 * 1024),
att.id
),
);
continue;
}
}

match download_slack_file(url) {
Ok(bytes) => {
// Post-download size guard: metadata size_bytes is optional,
// so a file with no size info could bypass the pre-download check.
if bytes.len() as u64 > MAX_DOWNLOAD_SIZE_BYTES {
channel_host::log(
channel_host::LogLevel::Warn,
&format!(
"Discarding Slack file after download: {} bytes exceeds {} MB limit (id={})",
bytes.len(),
MAX_DOWNLOAD_SIZE_BYTES / (1024 * 1024),
att.id
),
);
continue;
}

channel_host::log(
channel_host::LogLevel::Info,
&format!(
"Downloaded Slack file: {} bytes, mime={}",
bytes.len(),
att.mime_type
),
);
if let Err(e) = channel_host::store_attachment_data(&att.id, &bytes) {
Comment on lines +397 to +438

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

download_and_store_slack_files only skips downloads when att.size_bytes is present and over the limit. If Slack omits size (or reports incorrectly), this may download arbitrarily large files into WASM memory. Add a post-download check on bytes.len() (and skip/log if it exceeds the limit) to ensure the cap is enforced even when metadata is missing.

Copilot uses AI. Check for mistakes.
channel_host::log(
channel_host::LogLevel::Error,
&format!("Failed to store Slack file data: {}", e),
);
}
}
Err(e) => {
channel_host::log(
channel_host::LogLevel::Error,
&format!("Failed to download Slack file: {}", e),
);
}
}
}
Comment on lines +382 to +452

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slack attachments are downloaded unconditionally; if att.size_bytes is present and exceeds the host’s per-attachment storage limit, this will still download the entire file only to have store_attachment_data fail. Consider skipping downloads above the maximum storable size (or adding a conservative cap when size is unknown) to reduce bandwidth/memory risk inside the WASM component.

Copilot uses AI. Check for mistakes.
}

/// Handle a Slack event and emit message if applicable.
fn handle_slack_event(event: SlackEvent, team_id: Option<String>, _event_id: Option<String>) {
let attachments = extract_slack_attachments(&event.files);

// Download and store file attachments for host-side processing
download_and_store_slack_files(&attachments);

match event.event_type.as_str() {
// Direct mention of the bot (always in a channel, not a DM)
"app_mention" => {
Expand Down Expand Up @@ -722,4 +820,10 @@ mod tests {
let event: SlackEvent = serde_json::from_str(json).unwrap();
assert!(event.files.is_none());
}

#[test]
fn test_max_download_size_constant() {
// Verify the constant is 20 MB
assert_eq!(MAX_DOWNLOAD_SIZE_BYTES, 20 * 1024 * 1024);
}
}
2 changes: 1 addition & 1 deletion channels-src/telegram/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion channels-src/telegram/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "telegram-channel"
version = "0.2.0"
version = "0.2.1"
edition = "2021"
description = "Telegram Bot API channel for IronClaw"
license = "MIT OR Apache-2.0"
Expand Down
62 changes: 57 additions & 5 deletions channels-src/telegram/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -878,10 +878,6 @@ fn send_message(
// Voice File Download
// ============================================================================

/// Download a voice file from Telegram by file_id.
///
/// 1. Call getFile to get the file_path.
/// 2. Download the file bytes from /file/bot{TOKEN}/{file_path}.
/// Percent-encode a string for safe use as a URL query parameter value.
fn percent_encode(s: &str) -> String {
let mut out = String::with_capacity(s.len());
Expand All @@ -898,6 +894,10 @@ fn percent_encode(s: &str) -> String {
out
}

/// Maximum file size to download (20 MB). Files larger than this are discarded
/// to avoid excessive memory use and slow downloads in the WASM runtime.
const MAX_DOWNLOAD_SIZE_BYTES: u64 = 20 * 1024 * 1024;

fn download_telegram_file(file_id: &str) -> Result<Vec<u8>, String> {
// Reject file_id containing curly braces to prevent credential placeholder injection
if file_id.contains('{') || file_id.contains('}') {
Expand Down Expand Up @@ -965,6 +965,16 @@ fn download_telegram_file(file_id: &str) -> Result<Vec<u8>, String> {
));
}

// Post-download size guard: Telegram metadata file_size is optional,
// so enforce the limit on actual downloaded bytes.
if response.body.len() as u64 > MAX_DOWNLOAD_SIZE_BYTES {
return Err(format!(
"Downloaded file exceeds {} MB limit ({} bytes)",
MAX_DOWNLOAD_SIZE_BYTES / (1024 * 1024),
response.body.len()
));
}

Ok(response.body)
}

Expand Down Expand Up @@ -1535,6 +1545,39 @@ fn download_and_store_voice(attachments: &[InboundAttachment]) {
}
}

/// Download image file bytes and store them via the host for the vision pipeline.
///
/// Separated from `extract_attachments` so that function stays pure (no host
/// calls) and remains testable in native unit tests.
fn download_and_store_images(attachments: &[InboundAttachment]) {
for att in attachments {
if !att.mime_type.starts_with("image/") {
continue;
}

match download_telegram_file(&att.id) {
Ok(bytes) => {
channel_host::log(
channel_host::LogLevel::Info,
&format!("Downloaded image file: {} bytes", bytes.len()),
);
if let Err(e) = channel_host::store_attachment_data(&att.id, &bytes) {
channel_host::log(
channel_host::LogLevel::Error,
&format!("Failed to store image data: {}", e),
);
}
Comment on lines +1552 to +1569

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

download_and_store_images downloads every image/* attachment without checking size_bytes first. Large Telegram images can cause high memory use in the WASM runtime before host-side limits reject them. Add a size pre-check using att.size_bytes (and/or a post-download bytes.len() check) with a clear max (similar to Slack’s 20MB cap) to avoid OOM/slowdowns.

Copilot uses AI. Check for mistakes.
}
Err(e) => {
channel_host::log(
channel_host::LogLevel::Error,
&format!("Failed to download image file: {}", e),
);
}
}
}
}
Comment on lines +1552 to +1579

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This function is very similar to download_and_store_voice and download_and_store_documents. Having separate functions for each attachment type leads to code duplication and multiple iterations over the attachments list in handle_message.

To improve maintainability and performance, consider consolidating the download logic into a single function or a single loop in handle_message that dispatches based on attachment type. This would avoid iterating over the attachments multiple times.

References
  1. Consolidate related sequences of operations, such as creating, persisting, and scheduling a job, into a single reusable method to improve code consistency and maintainability.

Comment on lines +1548 to +1579

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Telegram image attachments are downloaded unconditionally. If att.size_bytes is available (or can be derived from Telegram metadata), consider skipping downloads above the host’s per-attachment storage limit to avoid unnecessary bandwidth/memory use when the host will reject oversized store_attachment_data calls anyway.

Copilot uses AI. Check for mistakes.

/// Returns true if the attachment should be downloaded for document text extraction.
///
/// Excludes voice (handled by transcription), image (vision pipeline),
Expand Down Expand Up @@ -1608,6 +1651,9 @@ fn handle_message(message: TelegramMessage) {
// Download and store voice attachments for host-side transcription
download_and_store_voice(&attachments);

// Download and store image attachments for host-side vision pipeline
download_and_store_images(&attachments);

// Download and store document attachments for host-side text extraction
download_and_store_documents(&mut attachments);

Expand Down Expand Up @@ -1681,7 +1727,7 @@ fn handle_message(message: TelegramMessage) {
let username_opt = from.username.as_deref();
let is_allowed = allowed.contains(&"*".to_string())
|| allowed.contains(&id_str)
|| username_opt.map_or(false, |u| allowed.contains(&u.to_string()));
|| username_opt.is_some_and(|u| allowed.contains(&u.to_string()));

if !is_allowed {
if is_private && dm_policy == "pairing" {
Expand Down Expand Up @@ -2605,4 +2651,10 @@ mod tests {
assert!(!is_downloadable_document(&make("audio/mpeg", Some("song.mp3"))));
assert!(!is_downloadable_document(&make("video/mp4", Some("clip.mp4"))));
}

#[test]
fn test_max_download_size_constant() {
// Verify the constant is 20 MB, matching the Slack channel limit
assert_eq!(MAX_DOWNLOAD_SIZE_BYTES, 20 * 1024 * 1024);
}
}
2 changes: 1 addition & 1 deletion registry/channels/slack.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "slack",
"display_name": "Slack Channel",
"kind": "channel",
"version": "0.2.0",
"version": "0.2.1",
"wit_version": "0.3.0",
"description": "Talk to your agent in Slack",
"keywords": [
Expand Down
2 changes: 1 addition & 1 deletion registry/channels/telegram.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "telegram",
"display_name": "Telegram Channel",
"kind": "channel",
"version": "0.2.0",
"version": "0.2.1",
"wit_version": "0.3.0",
"description": "Talk to your agent through a Telegram bot",
"keywords": [
Expand Down
92 changes: 90 additions & 2 deletions src/agent/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,8 +681,53 @@ impl Agent {
.into())
});

// Send ToolResult preview
if let Ok(ref output) = tool_result
// Detect image generation sentinel in tool output
// (only from image tools β€” avoids parsing all tool outputs)
let is_image_sentinel = if let Ok(ref output) = tool_result
&& matches!(tc.name.as_str(), "image_generate" | "image_edit")
{
if let Ok(sentinel) =
serde_json::from_str::<serde_json::Value>(output)
&& sentinel.get("type").and_then(|v| v.as_str())
== Some("image_generated")
{
let data_url = sentinel
.get("data")
.and_then(|v| v.as_str())
.unwrap_or_default()
.to_string();
let path = sentinel
.get("path")
.and_then(|v| v.as_str())
.map(String::from);
// Skip broadcasting if data_url is empty to avoid
// sending a broken ImageGenerated SSE event.
if data_url.is_empty() {
tracing::warn!(
"Image generation sentinel has empty data URL, skipping broadcast"
);
} else {
let _ = self
.channels
.send_status(
&message.channel,
StatusUpdate::ImageGenerated { data_url, path },
&message.metadata,
)
.await;
}
true
} else {
false
}
} else {
false
};

// Send ToolResult preview (skip for image sentinels to avoid
// broadcasting multi-MB base64 data as a preview)
if !is_image_sentinel
&& let Ok(ref output) = tool_result
&& !output.is_empty()
{
let _ = self
Expand Down Expand Up @@ -2124,4 +2169,47 @@ mod tests {
"Error should include the underlying reason, got: {formatted}"
);
}

#[test]
fn test_image_sentinel_empty_data_url_should_be_skipped() {
// Regression: unwrap_or_default() on missing "data" field produces an empty
// string. Broadcasting an empty data_url would send a broken SSE event.
let sentinel = serde_json::json!({
"type": "image_generated",
"path": "/tmp/image.png"
// "data" field is missing
});

let data_url = sentinel
.get("data")
.and_then(|v| v.as_str())
.unwrap_or_default()
.to_string();

assert!(
data_url.is_empty(),
"Missing 'data' field should produce empty string"
);
// The fix: empty data_url means we skip broadcasting
}

#[test]
fn test_image_sentinel_present_data_url_is_valid() {
let sentinel = serde_json::json!({
"type": "image_generated",
"data": "data:image/png;base64,abc123",
"path": "/tmp/image.png"
});

let data_url = sentinel
.get("data")
.and_then(|v| v.as_str())
.unwrap_or_default()
.to_string();

assert!(
!data_url.is_empty(),
"Present 'data' field should produce non-empty string"
);
}
}
Loading
Loading