fix(telegram): resolve send_attachment paths in the user's workspace
Nightly Build / build (push) Successful in 8m6s

`send_attachment` handed its `file_path` argument straight to
`InputFile::file`, which resolves against the **server process's** working
directory. Every path the model can actually have — relative to the user's
home, or absolute inside their container — failed the `path.exists()` check,
and the one class that didn't (a name that happens to exist next to the
binary) would have sent the wrong file.

The routing already exists for the fs-tools, so expose it rather than repeat
it: `UserFilesApi` (core-api) reads a path in the agent's own vocabulary and
is obtained from `UserChannelHandle::files()`, so it is scoped to one user by
construction. skald-core implements it over `resolve_view_target` — host
mount read directly, container-only path through `docker exec` — holding the
`SharedFs` cell rather than a snapshot, so a remount lands without a login.

The size cap is checked before the read (a new `exec_fs::size` for the
container branch): the point of a cap is to keep an oversized file out of RAM,
so checking it afterwards would protect nothing. A photo above `sendPhoto`'s
narrower 10 MB ceiling goes out as a document instead of as an API error.
This commit is contained in:
Daniele
2026-08-10 00:08:16 +01:00
parent 5765941758
commit 55dcb48299
7 changed files with 171 additions and 12 deletions
+1
View File
@@ -22,6 +22,7 @@ pub mod provider;
pub mod remote; pub mod remote;
pub mod tool; pub mod tool;
pub mod user_channel; pub mod user_channel;
pub mod user_files;
pub mod user_fs; pub mod user_fs;
pub mod user_plugin_config; pub mod user_plugin_config;
pub mod secrets; pub mod secrets;
+8
View File
@@ -23,6 +23,7 @@ use crate::approval::ApprovalApi;
use crate::chat_hub::ChatHubApi; use crate::chat_hub::ChatHubApi;
use crate::events::GlobalEvent; use crate::events::GlobalEvent;
use crate::inbox::InboxApi; use crate::inbox::InboxApi;
use crate::user_files::UserFilesApi;
/// Resolves an unlocked user's channel handle. /// Resolves an unlocked user's channel handle.
/// ///
@@ -84,6 +85,13 @@ pub trait UserChannelHandle: Send + Sync {
/// `approval()`/clarification/elicitation separately. /// `approval()`/clarification/elicitation separately.
fn inbox(&self) -> Arc<dyn InboxApi>; fn inbox(&self) -> Arc<dyn InboxApi>;
/// The user's workspace files — reading a path in the agent's own vocabulary
/// (`~/…`, `shared/{X}/…`, `/tmp/…`), routed to the host mount or to the
/// container exactly as the fs-tools route it. A channel adapter that sends a
/// file back to the user goes through this rather than the host filesystem,
/// whose cwd is the server's and not the user's.
fn files(&self) -> Arc<dyn UserFilesApi>;
/// Subscribe to the user's server→client event stream. /// Subscribe to the user's server→client event stream.
/// Events are scoped to this user; no cross-user leakage. /// Events are scoped to this user; no cross-user leakage.
fn subscribe(&self) -> broadcast::Receiver<GlobalEvent>; fn subscribe(&self) -> broadcast::Receiver<GlobalEvent>;
+42
View File
@@ -0,0 +1,42 @@
//! Reading a user's files from a channel plugin (blueprint §6).
//!
//! A channel adapter that hands a file back to the user — Telegram's
//! `send_attachment` is the first — is given a path in the **agent's** vocabulary
//! (`~/report.pdf`, `uploads/{session}/photo.jpg`, `shared/{X}/…`, or a
//! container-absolute `/tmp/out.png`), because that is the only vocabulary the
//! model has ever seen. None of those spellings is a host path: resolving them
//! means the same two-backing routing the fs-tools do — a bind-mounted path read
//! host-side, anything else read through the user's container.
//!
//! That routing lives in the core, so this is the seam that lets a plugin borrow
//! it instead of touching the process working directory (which is what a plain
//! `std::fs::read` of an agent path does — it either fails or, worse, reads a
//! same-named file next to the binary).
use async_trait::async_trait;
/// A file read out of a user's workspace.
pub struct UserFile {
/// The canonical agent-vocabulary path — what the user and the model see.
pub display: String,
/// Basename of [`display`](Self::display), for surfaces that need a file name.
pub name: String,
pub bytes: Vec<u8>,
}
/// Reads files from one user's workspace, with the agent's own path routing.
///
/// Obtained from [`UserChannelHandle::files`](crate::user_channel::UserChannelHandle::files),
/// so it is already scoped to that user: containment is the core's
/// (canonicalize + prefix-check on the mounts, the container otherwise) and a
/// path outside the caller's view is refused, never silently resolved elsewhere.
#[async_trait]
pub trait UserFilesApi: Send + Sync {
/// Reads `path`, refusing anything larger than `max_bytes` **before** loading
/// it — the cap is the caller's own limit (Telegram's upload ceiling, say),
/// and a size check that ran after the read would protect nothing.
///
/// Virtual memory notes (`user-memory/…`, `shared-memory/…`) are not files and
/// are rejected with a clear error.
async fn read(&self, path: &str, max_bytes: u64) -> anyhow::Result<UserFile>;
}
+3 -1
View File
@@ -418,7 +418,9 @@ async fn handle_llm_message(
client_name, client_name,
extra_system_context: Some(TELEGRAM_FORMAT_CONTEXT.to_string()), extra_system_context: Some(TELEGRAM_FORMAT_CONTEXT.to_string()),
tail_reminder: Some(super::TELEGRAM_FORMAT_REMINDER.to_string()), tail_reminder: Some(super::TELEGRAM_FORMAT_REMINDER.to_string()),
interface_tools: super::tools::interface_tools(bot.clone(), chat_id, &*shared.tts).await, interface_tools: super::tools::interface_tools(
bot.clone(), chat_id, &*shared.tts, handle.files(),
).await,
metadata, metadata,
..Default::default() ..Default::default()
}; };
+39 -11
View File
@@ -8,6 +8,7 @@ use teloxide::types::InputFile;
use core_api::interface_tool::InterfaceTool; use core_api::interface_tool::InterfaceTool;
use core_api::tool::{Tool, ToolCategory, ToolDescriptionLength}; use core_api::tool::{Tool, ToolCategory, ToolDescriptionLength};
use core_api::tts::{TextToSpeech, TtsProvider}; use core_api::tts::{TextToSpeech, TtsProvider};
use core_api::user_files::UserFilesApi;
use super::auth::{Binding, load_config, save_config}; use super::auth::{Binding, load_config, save_config};
use super::TelegramPlugin; use super::TelegramPlugin;
@@ -26,8 +27,9 @@ pub(crate) async fn interface_tools(
bot: Bot, bot: Bot,
chat_id: ChatId, chat_id: ChatId,
tts: &dyn TtsProvider, tts: &dyn TtsProvider,
files: Arc<dyn UserFilesApi>,
) -> Vec<InterfaceTool> { ) -> Vec<InterfaceTool> {
let mut tools = vec![send_attachment_tool(bot.clone(), chat_id)]; let mut tools = vec![send_attachment_tool(bot.clone(), chat_id, files)];
if let Some(synth) = tts.get().await { if let Some(synth) = tts.get().await {
tools.push(send_voice_tool(bot, chat_id, synth)); tools.push(send_voice_tool(bot, chat_id, synth));
@@ -38,19 +40,37 @@ pub(crate) async fn interface_tools(
// ── send_attachment ─────────────────────────────────────────────────────────── // ── send_attachment ───────────────────────────────────────────────────────────
fn send_attachment_tool(bot: Bot, chat_id: ChatId) -> InterfaceTool { /// What the Bot API accepts in one upload (50 MB). Checked before the file is
/// read, so an oversized one costs a `stat` rather than a rejected 50 MB POST.
const TELEGRAM_UPLOAD_LIMIT: u64 = 50 * 1000 * 1000;
/// The narrower ceiling `sendPhoto` enforces — above it an image is sent as a
/// document instead, which is the same bytes without the inline preview.
const TELEGRAM_PHOTO_LIMIT: u64 = 10 * 1000 * 1000;
/// Sends a file from the **user's** workspace, resolved through
/// [`UserFilesApi`] — the same routing the fs-tools use, so `~/report.pdf`,
/// `uploads/{session}/photo.jpg` and the container-only `/tmp/out.png` all work.
///
/// It used to hand the raw argument to `InputFile::file`, which resolves against
/// the **server process's** working directory: every agent path the model has
/// ever been given (each of them relative to the user's home, or absolute inside
/// their container) failed the `path.exists()` check, and the one class that did
/// not — a name that happens to exist next to the binary — would have sent the
/// wrong file entirely.
fn send_attachment_tool(bot: Bot, chat_id: ChatId, files: Arc<dyn UserFilesApi>) -> InterfaceTool {
InterfaceTool { InterfaceTool {
definition: json!({ definition: json!({
"type": "function", "type": "function",
"function": { "function": {
"name": "send_attachment", "name": "send_attachment",
"description": "Send a file from the local filesystem to the user on Telegram. Images (jpg/png/webp) and videos (mp4/mov/webm) are sent inline by default; any other type is sent as a document. Set as_document=true to force sending as a downloadable file.", "description": "Send a file to the user on Telegram. Images (jpg/png/webp) and videos (mp4/mov/webm) are sent inline by default; any other type is sent as a document. Set as_document=true to force sending as a downloadable file.",
"parameters": { "parameters": {
"type": "object", "type": "object",
"properties": { "properties": {
"file_path": { "file_path": {
"type": "string", "type": "string",
"description": "Absolute or relative path to the file to send." "description": "Path to the file, in your usual vocabulary: `~/report.pdf`, `uploads/…`, `shared/{folder}/…`, `projects/…`, or an absolute path inside your sandbox (`/tmp/out.png`). Memory notes cannot be sent."
}, },
"caption": { "caption": {
"type": "string", "type": "string",
@@ -67,6 +87,7 @@ fn send_attachment_tool(bot: Bot, chat_id: ChatId) -> InterfaceTool {
}), }),
handler: Arc::new(move |args| { handler: Arc::new(move |args| {
let bot = bot.clone(); let bot = bot.clone();
let files = Arc::clone(&files);
Box::pin(async move { Box::pin(async move {
let file_path = args["file_path"] let file_path = args["file_path"]
.as_str() .as_str()
@@ -74,18 +95,17 @@ fn send_attachment_tool(bot: Bot, chat_id: ChatId) -> InterfaceTool {
let caption = args["caption"].as_str().map(str::to_string); let caption = args["caption"].as_str().map(str::to_string);
let as_document = args["as_document"].as_bool().unwrap_or(false); let as_document = args["as_document"].as_bool().unwrap_or(false);
let path = std::path::Path::new(file_path); let read = files.read(file_path, TELEGRAM_UPLOAD_LIMIT).await
if !path.exists() { .map_err(|e| anyhow::anyhow!("send_attachment: {e}"))?;
anyhow::bail!("send_attachment: file not found: {file_path}");
}
// Present images/videos inline by default; everything else (and // Present images/videos inline by default; everything else (and
// anything when as_document=true) as a downloadable document. // anything when as_document=true) as a downloadable document.
let ext = path.extension() let ext = std::path::Path::new(&read.name)
.extension()
.and_then(|e| e.to_str()) .and_then(|e| e.to_str())
.unwrap_or("") .unwrap_or("")
.to_ascii_lowercase(); .to_ascii_lowercase();
let kind = if as_document { let mut kind = if as_document {
"document" "document"
} else { } else {
match ext.as_str() { match ext.as_str() {
@@ -94,8 +114,16 @@ fn send_attachment_tool(bot: Bot, chat_id: ChatId) -> InterfaceTool {
_ => "document", _ => "document",
} }
}; };
// `sendPhoto` caps at 10 MB where `sendDocument` takes 50, so a big
// image goes out as a file rather than as an API error.
if kind == "photo" && read.bytes.len() as u64 > TELEGRAM_PHOTO_LIMIT {
kind = "document";
}
let file = InputFile::file(path); // The bytes are already in hand — a container file has no host path
// to point Telegram at, and a mounted one would only be re-read.
let file = InputFile::memory(read.bytes).file_name(read.name);
let file_path = read.display;
let result = match kind { let result = match kind {
"photo" => { "photo" => {
let mut req = bot.send_photo(chat_id, file); let mut req = bot.send_photo(chat_id, file);
@@ -93,6 +93,20 @@ pub async fn write(container: &str, path: &Path, bytes: &[u8]) -> Result<()> {
Ok(()) Ok(())
} }
/// Byte size of a file inside the container — for the callers that must decide
/// whether to read it *before* pulling it through the pipe. `wc -c` rather than
/// `stat`, so the answer is the same on any of the image's shells.
pub async fn size(container: &str, path: &Path) -> Result<u64> {
let p = path.to_string_lossy();
let raw = sh(container, r#"wc -c < "$1""#, &[&p])
.await
.with_context(|| format!("Cannot stat file: {p}"))?;
String::from_utf8_lossy(&raw)
.trim()
.parse()
.with_context(|| format!("Cannot stat file: {p}"))
}
pub async fn exists(container: &str, path: &Path) -> bool { pub async fn exists(container: &str, path: &Path) -> bool {
sh_ok(container, r#"test -e "$1""#, &[&path.to_string_lossy()]).await sh_ok(container, r#"test -e "$1""#, &[&path.to_string_lossy()]).await
} }
@@ -35,6 +35,7 @@ use core_api::events::GlobalEvent;
use core_api::inbox::InboxApi; use core_api::inbox::InboxApi;
use core_api::system_bus::SystemEventBus; use core_api::system_bus::SystemEventBus;
use core_api::user_channel::UserChannelHandle; use core_api::user_channel::UserChannelHandle;
use core_api::user_files::{UserFile, UserFilesApi};
use core_api::user_fs::SharedFs; use core_api::user_fs::SharedFs;
use crate::approval::ApprovalManager; use crate::approval::ApprovalManager;
@@ -560,7 +561,70 @@ impl UserChannelHandle for UserContextHandle {
Arc::new(self.ctx.inbox.clone()) as Arc<dyn InboxApi> Arc::new(self.ctx.inbox.clone()) as Arc<dyn InboxApi>
} }
fn files(&self) -> Arc<dyn UserFilesApi> {
Arc::new(UserContextFiles { fs: self.ctx.fs.clone() }) as Arc<dyn UserFilesApi>
}
fn subscribe(&self) -> broadcast::Receiver<GlobalEvent> { fn subscribe(&self) -> broadcast::Receiver<GlobalEvent> {
self.ctx.global_tx.subscribe() self.ctx.global_tx.subscribe()
} }
} }
// ── UserFilesApi impl ─────────────────────────────────────────────────────────
/// Reads one user's files for a channel plugin, with the fs-tools' own routing.
///
/// It holds the [`SharedFs`] rather than a snapshot of it, so a membership change
/// that remounts the user's container (§6) is picked up on the next read instead
/// of at the next login.
struct UserContextFiles {
fs: SharedFs,
}
#[async_trait::async_trait]
impl UserFilesApi for UserContextFiles {
async fn read(&self, path: &str, max_bytes: u64) -> Result<UserFile> {
let fs = self.fs.load();
let (target, display) = crate::tools::fs::resolve_view_target(fs.as_ref(), path)?;
let name = std::path::Path::new(&display)
.file_name()
.map(|n| n.to_string_lossy().into_owned())
.unwrap_or_else(|| display.clone());
// Size first, in both branches: the cap exists to keep an oversized file
// out of RAM, so checking it after the read would be decoration.
let too_big = |size: u64| {
anyhow::anyhow!(
"{display} is {:.1} MB — larger than the {:.0} MB this can send",
size as f64 / 1e6,
max_bytes as f64 / 1e6,
)
};
let bytes = match target {
crate::tools::fs::FsTarget::Host(abs) => {
let meta = tokio::fs::metadata(&abs)
.await
.map_err(|_| anyhow::anyhow!("file not found: {display}"))?;
if meta.is_dir() {
anyhow::bail!("{display} is a directory, not a file");
}
if meta.len() > max_bytes {
anyhow::bail!(too_big(meta.len()));
}
tokio::fs::read(&abs).await?
}
crate::tools::fs::FsTarget::Container { container, path } => {
let size = crate::container::exec_fs::size(&container, &path)
.await
.map_err(|_| anyhow::anyhow!("file not found: {display}"))?;
if size > max_bytes {
anyhow::bail!(too_big(size));
}
crate::container::exec_fs::read(&container, &path).await?
}
};
Ok(UserFile { display, name, bytes })
}
}