From 25728b71c57241326c15d45df7a67b9addb3d190 Mon Sep 17 00:00:00 2001 From: Jason T Alborough Date: Fri, 26 Jun 2026 09:32:11 -0400 Subject: [PATCH] fix(chat): require per-channel sender allowlist for inbound commands (#1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(chat): require per-channel sender allowlist for inbound commands Chat-channel commands (/task, /folder, /approve, …) spawn and drive agents with the host's full privileges, but the dispatcher routed purely on the command verb — sender_id was logged and used for context yet never authorized. Any user who could message the bot could run arbitrary tasks (RCE) and /approve always disabled the human-in-the-loop. This is acute for an always-on server reachable via Telegram. Add a fail-closed authorization gate at the top of dispatch_command (covers Telegram, Lark, and WeChat in one place): only sender ids listed in the channel's config_json `allowed_senders` may drive the bot. An empty/unset list authorizes no one; a blocked sender is told their own id so the operator can add it. Surfaced in the add/edit channel dialogs as an "Allowed Sender IDs" field. - chat_channel/authz.rs: pure is_sender_allowed() + unit tests - command_dispatcher.rs: gate before any command, incl. follow-ups - i18n.rs: unauthorized reply (10 languages) - add/edit-chat-channel-dialog.tsx: allowlist management UI Co-Authored-By: Claude Opus 4.8 (1M context) * fix(chat): annotate allowedSenders types for strict tsc `pnpm build` (strict tsc) flagged the `.map((s) => …)` callback as implicit `any`: in the edit dialog `config` comes from `JSON.parse` (any), so the `allowedSenders` state inferred `any`. Type the state as `string` and annotate the map parameter. eslint doesn't run the type-checker so it passed locally; validated now with `pnpm build` + `pnpm test` (1747 passing). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- src-tauri/src/chat_channel/authz.rs | 96 +++++++++++++++++++ .../src/chat_channel/command_dispatcher.rs | 35 ++++++- src-tauri/src/chat_channel/i18n.rs | 52 ++++++++++ src-tauri/src/chat_channel/mod.rs | 1 + .../settings/add-chat-channel-dialog.tsx | 38 +++++++- .../settings/edit-chat-channel-dialog.tsx | 41 +++++++- 6 files changed, 252 insertions(+), 11 deletions(-) create mode 100644 src-tauri/src/chat_channel/authz.rs diff --git a/src-tauri/src/chat_channel/authz.rs b/src-tauri/src/chat_channel/authz.rs new file mode 100644 index 000000000..211bc93b1 --- /dev/null +++ b/src-tauri/src/chat_channel/authz.rs @@ -0,0 +1,96 @@ +//! Inbound sender authorization for chat channels. +//! +//! Chat commands can spawn and drive real agents (`/task`, `/approve`, …) with the +//! host's full privileges, so an un-vetted inbound message is equivalent to remote +//! code execution. Every inbound command is therefore gated on a per-channel +//! allowlist of sender ids, stored in the channel `config_json` under +//! `allowed_senders` (an array of strings). +//! +//! The gate is **fail-closed**: a missing key, an empty list, or an unparseable +//! config authorizes nobody. Operators add their sender id (surfaced to them in the +//! "unauthorized" reply) via the channel settings before the bot will act. + +use serde_json::Value; + +/// Extract and normalize the `allowed_senders` list from a channel `config_json`. +/// Entries are trimmed; blanks are dropped. Returns empty on any parse failure. +fn allowed_senders(config_json: &str) -> Vec { + serde_json::from_str::(config_json) + .ok() + .as_ref() + .and_then(|v| v.get("allowed_senders")) + .and_then(Value::as_array) + .map(|arr| { + arr.iter() + .filter_map(Value::as_str) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .collect() + }) + .unwrap_or_default() +} + +/// Whether `sender_id` is permitted to drive the channel. +/// +/// Fail-closed: an empty/absent allowlist or a blank sender authorizes no one. +pub fn is_sender_allowed(config_json: &str, sender_id: &str) -> bool { + let sender = sender_id.trim(); + if sender.is_empty() { + return false; + } + allowed_senders(config_json) + .iter() + .any(|allowed| allowed == sender) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn empty_or_missing_allowlist_denies_everyone() { + assert!(!is_sender_allowed("{}", "123")); + assert!(!is_sender_allowed( + r#"{"bot_token":"x","chat_id":"y"}"#, + "123" + )); + assert!(!is_sender_allowed(r#"{"allowed_senders":[]}"#, "123")); + } + + #[test] + fn unparseable_config_denies() { + assert!(!is_sender_allowed("not json", "123")); + assert!(!is_sender_allowed("", "123")); + } + + #[test] + fn listed_sender_is_allowed_others_are_not() { + let cfg = r#"{"bot_token":"x","allowed_senders":["123","456"]}"#; + assert!(is_sender_allowed(cfg, "123")); + assert!(is_sender_allowed(cfg, "456")); + assert!(!is_sender_allowed(cfg, "789")); + } + + #[test] + fn ids_and_sender_are_trimmed() { + let cfg = r#"{"allowed_senders":[" 123 ","456"]}"#; + assert!(is_sender_allowed(cfg, "123")); + assert!(is_sender_allowed(cfg, " 123 ")); + } + + #[test] + fn blank_sender_is_denied_even_if_blank_listed() { + // blanks are stripped from the list, so an empty sender never matches + let cfg = r#"{"allowed_senders":[""," "]}"#; + assert!(!is_sender_allowed(cfg, "")); + assert!(!is_sender_allowed(cfg, " ")); + } + + #[test] + fn non_string_entries_are_ignored() { + let cfg = r#"{"allowed_senders":[123,"456",true,null]}"#; + // numeric 123 is ignored (Telegram ids arrive as strings); only "456" counts + assert!(!is_sender_allowed(cfg, "123")); + assert!(is_sender_allowed(cfg, "456")); + } +} diff --git a/src-tauri/src/chat_channel/command_dispatcher.rs b/src-tauri/src/chat_channel/command_dispatcher.rs index 89ba11e64..005aafe46 100644 --- a/src-tauri/src/chat_channel/command_dispatcher.rs +++ b/src-tauri/src/chat_channel/command_dispatcher.rs @@ -5,6 +5,7 @@ use sea_orm::DatabaseConnection; use tokio::sync::{mpsc, Mutex}; use tokio::task::JoinHandle; +use super::authz; use super::command_handlers; use super::i18n::{self, Lang}; use super::manager::ChatChannelManager; @@ -12,7 +13,9 @@ use super::session_bridge::SessionBridge; use super::session_commands; use super::types::IncomingCommand; use crate::acp::manager::ConnectionManager; -use crate::db::service::{app_metadata_service, chat_channel_message_log_service}; +use crate::db::service::{ + app_metadata_service, chat_channel_message_log_service, chat_channel_service, +}; use crate::web::event_bridge::EventEmitter; const COMMAND_PREFIX_KEY: &str = "chat_command_prefix"; @@ -68,7 +71,9 @@ pub fn spawn_command_dispatcher( let text = cmd.command_text.trim(); tracing::info!( "[ChatChannel] received command from channel={} sender={}: {:?}", - cmd.channel_id, cmd.sender_id, text + cmd.channel_id, + cmd.sender_id, + text ); // Log inbound command @@ -112,7 +117,8 @@ pub fn spawn_command_dispatcher( Err(e) => { tracing::error!( "[ChatChannel] failed to send response for {:?} to channel {}: {e}", - text, cmd.channel_id + text, + cmd.channel_id ); ("failed", Some(e.to_string())) } @@ -145,6 +151,29 @@ async fn dispatch_command( sender_id: &str, lang: Lang, ) -> super::types::RichMessage { + // ── Authorization gate (fail-closed) ── + // Chat commands spawn and drive agents with the host's full privileges, so + // every inbound message must come from an allow-listed sender. An empty or + // unset allowlist authorizes no one. We reply with the sender's own id (and + // run no command) so the operator can add it from the channel settings. + let authorized = match chat_channel_service::get_by_id(db, channel_id).await { + Ok(Some(ch)) => authz::is_sender_allowed(&ch.config_json, sender_id), + Ok(None) => false, + Err(e) => { + tracing::error!( + "[ChatChannel] authz lookup failed for channel={channel_id}: {e}; denying" + ); + false + } + }; + if !authorized { + tracing::warn!( + "[ChatChannel] BLOCKED unauthorized sender={sender_id} on channel={channel_id}: {text:?}" + ); + return super::types::RichMessage::error(i18n::unauthorized_sender_body(lang, sender_id)) + .with_title(i18n::unauthorized_sender_title(lang)); + } + // Strip prefix; if text doesn't start with it, try as follow-up let without_prefix = match text.strip_prefix(prefix) { Some(rest) => rest, diff --git a/src-tauri/src/chat_channel/i18n.rs b/src-tauri/src/chat_channel/i18n.rs index f674f691f..910d24d92 100644 --- a/src-tauri/src/chat_channel/i18n.rs +++ b/src-tauri/src/chat_channel/i18n.rs @@ -816,6 +816,58 @@ pub fn unknown_command_title(lang: Lang) -> &'static str { } } +pub fn unauthorized_sender_title(lang: Lang) -> &'static str { + match lang { + Lang::ZhCn => "未授权", + Lang::ZhTw => "未授權", + Lang::Ja => "未承認", + Lang::Ko => "권한 없음", + Lang::Es => "No autorizado", + Lang::De => "Nicht autorisiert", + Lang::Fr => "Non autorisé", + Lang::Pt => "Não autorizado", + Lang::Ar => "غير مصرح", + Lang::En => "Unauthorized", + } +} + +/// Reply sent to a sender who is not on the channel allowlist. Includes their own +/// sender id so the operator can add it to the channel's allowed senders. +pub fn unauthorized_sender_body(lang: Lang, sender_id: &str) -> String { + match lang { + Lang::ZhCn => format!( + "你无权使用此机器人。你的发送者 ID 是 `{sender_id}`。请管理员将其加入该频道的允许发送者列表。" + ), + Lang::ZhTw => format!( + "你無權使用此機器人。你的發送者 ID 是 `{sender_id}`。請管理員將其加入該頻道的允許發送者清單。" + ), + Lang::Ja => format!( + "このボットを使用する権限がありません。あなたの送信者 ID は `{sender_id}` です。管理者にチャンネルの許可リストへの追加を依頼してください。" + ), + Lang::Ko => format!( + "이 봇을 사용할 권한이 없습니다. 발신자 ID는 `{sender_id}` 입니다. 관리자에게 채널 허용 목록에 추가를 요청하세요." + ), + Lang::Es => format!( + "No tienes autorización para usar este bot. Tu ID de remitente es `{sender_id}`. Pide al administrador que lo añada a los remitentes permitidos del canal." + ), + Lang::De => format!( + "Du bist nicht berechtigt, diesen Bot zu nutzen. Deine Sender-ID ist `{sender_id}`. Bitte den Administrator, sie zur Liste der erlaubten Sender des Kanals hinzuzufügen." + ), + Lang::Fr => format!( + "Vous n'êtes pas autorisé à utiliser ce bot. Votre identifiant d'expéditeur est `{sender_id}`. Demandez à l'administrateur de l'ajouter aux expéditeurs autorisés du canal." + ), + Lang::Pt => format!( + "Você não tem autorização para usar este bot. Seu ID de remetente é `{sender_id}`. Peça ao administrador para adicioná-lo aos remetentes permitidos do canal." + ), + Lang::Ar => format!( + "غير مصرح لك باستخدام هذا البوت. معرّف المرسل الخاص بك هو `{sender_id}`. اطلب من المسؤول إضافته إلى قائمة المرسلين المسموح لهم في القناة." + ), + Lang::En => format!( + "You are not authorized to use this bot. Your sender ID is `{sender_id}`. Ask the administrator to add it to this channel's allowed senders." + ), + } +} + // ── Session command messages ── // Folder (/folder) diff --git a/src-tauri/src/chat_channel/mod.rs b/src-tauri/src/chat_channel/mod.rs index 3de0fb693..48960f73b 100644 --- a/src-tauri/src/chat_channel/mod.rs +++ b/src-tauri/src/chat_channel/mod.rs @@ -1,3 +1,4 @@ +pub mod authz; pub mod backends; pub mod command_dispatcher; pub mod command_handlers; diff --git a/src/components/settings/add-chat-channel-dialog.tsx b/src/components/settings/add-chat-channel-dialog.tsx index ea6b5c534..4b1fbec8a 100644 --- a/src/components/settings/add-chat-channel-dialog.tsx +++ b/src/components/settings/add-chat-channel-dialog.tsx @@ -6,6 +6,7 @@ import { useTranslations } from "next-intl" import { Button } from "@/components/ui/button" import { Input } from "@/components/ui/input" +import { Textarea } from "@/components/ui/textarea" import { Dialog, DialogContent, @@ -46,6 +47,7 @@ export function AddChatChannelDialog({ const [chatId, setChatId] = useState("") const [appId, setAppId] = useState("") const [baseUrl, setBaseUrl] = useState("https://ilinkai.weixin.qq.com") + const [allowedSenders, setAllowedSenders] = useState("") const [dailyReportEnabled, setDailyReportEnabled] = useState(false) const [dailyReportTime, setDailyReportTime] = useState("18:00") @@ -56,6 +58,7 @@ export function AddChatChannelDialog({ setChatId("") setAppId("") setBaseUrl("https://ilinkai.weixin.qq.com") + setAllowedSenders("") setDailyReportEnabled(false) setDailyReportTime("18:00") setError(null) @@ -86,12 +89,22 @@ export function AddChatChannelDialog({ setLoading(true) setError(null) try { - const configJson = + const allowedSendersArr = allowedSenders + .split("\n") + .map((s: string) => s.trim()) + .filter(Boolean) + + const baseConfig = channelType === "weixin" - ? JSON.stringify({ base_url: baseUrl }) + ? { base_url: baseUrl } : channelType === "lark" - ? JSON.stringify({ app_id: appId, chat_id: chatId }) - : JSON.stringify({ chat_id: chatId }) + ? { app_id: appId, chat_id: chatId } + : { chat_id: chatId } + + const configJson = JSON.stringify({ + ...baseConfig, + allowed_senders: allowedSendersArr, + }) const channel = await createChatChannel({ name: name.trim(), @@ -121,6 +134,7 @@ export function AddChatChannelDialog({ channelType, appId, baseUrl, + allowedSenders, dailyReportEnabled, dailyReportTime, handleOpenChange, @@ -208,6 +222,22 @@ export function AddChatChannelDialog({

)} +
+ +