From 5d081e25804889399f808010bfddb94cb0c653f4 Mon Sep 17 00:00:00 2001 From: xiaoxixi Date: Sun, 19 Jul 2026 15:42:01 +0800 Subject: [PATCH] fix(feishu): enforce inbound admission rules --- README.md | 3 +- docs/ARCHITECTURE.md | 2 +- .../about-picobot/assets/config.example.json | 1 + .../skills/about-picobot/references/config.md | 1 + resources/templates/config.example.json | 1 + src/channels/feishu.rs | 301 +++++++++++++++--- src/config/mod.rs | 3 + 7 files changed, 272 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 565a8d5..6186d8e 100644 --- a/README.md +++ b/README.md @@ -243,7 +243,7 @@ WebUI/TUI 上传文件默认保存到 `~/.picobot/media/cli_chat`，单文件上 | `cli_chat` | Ratatui 终端客户端，通过 WebSocket 连接 Gateway | | `feishu` | 飞书/Lark 消息、反应、文件上传下载和媒体引用 | -飞书默认只发送终态结果。设置 `channels.feishu.live_updates=true` 后会创建一张卡片并持续编辑；`live_update_interval_ms` 默认 500ms，运行时限制在 250–5000ms。外部渠道始终不会收到模型 reasoning。 +飞书默认只接受 `allow_from` 中的用户，且群聊消息必须明确 @ 机器人（可通过 `channels.feishu.require_mention=false` 关闭）。默认只发送终态结果；设置 `channels.feishu.live_updates=true` 后会创建一张卡片并持续编辑，`live_update_interval_ms` 默认 500ms，运行时限制在 250–5000ms。外部渠道始终不会收到模型 reasoning。 ### 会话 @@ -348,6 +348,7 @@ Skill 是包含 `SKILL.md` 的目录。加载优先级从高到低： | `browser.enabled` | `false` | | `channels.feishu.live_updates` | `false` | | `channels.feishu.live_update_interval_ms` | `500` | +| `channels.feishu.require_mention` | `true` | 更完整的配置字段说明见 [resources/skills/about-picobot/references/config.md](resources/skills/about-picobot/references/config.md)。 diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 79a1711..b26ba7e 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -139,7 +139,7 @@ sequenceDiagram - `LivePolicy::Snapshot` 按渠道间隔发送最新运行态；`FinalOnly` 忽略运行态，只处理终态。终态绕过节流并只对明确的瞬态错误重试。 - `TurnDeliveryService` 返回可等待的终态句柄；sink 生命周期启动不等于终态已送达。Session 在终态重试最终失败时通过普通出站路径兜底一次。 - `cli_chat` 将同一 `turn_updated` 快照发给 TUI 和 WebUI。客户端只保留当前 session 中 revision 更新的 `active_turn`，终态随后由持久化历史校准。 -- 飞书默认 `FinalOnly`；开启 `live_updates` 后，第一个可见快照创建卡片，后续编辑同一卡片，终态编辑失败则发送完整结果兜底。reaction 清理在 finish、abort 和 Gateway shutdown 中幂等执行。 +- 飞书在协议解析阶段按 `allow_from` 拒绝未授权用户；群聊默认必须明确 @ 运行时解析出的机器人身份，身份解析失败时安全地忽略群消息。飞书默认 `FinalOnly`；开启 `live_updates` 后，第一个可见快照创建卡片，后续编辑同一卡片，终态编辑失败则发送完整结果兜底。reaction 清理在 finish、abort 和 Gateway shutdown 中幂等执行。 - DeliveryCoordinator 与 OutboundDispatcher 共享 `(channel, chat_id)` 写锁，避免活动 Turn 终态与独立消息并发写入同一目标。 ### 出站投递 diff --git a/resources/skills/about-picobot/assets/config.example.json b/resources/skills/about-picobot/assets/config.example.json index c4dbbca..1084f6a 100644 --- a/resources/skills/about-picobot/assets/config.example.json +++ b/resources/skills/about-picobot/assets/config.example.json @@ -60,6 +60,7 @@ "app_id": "", "app_secret": "", "allow_from": ["*"], + "require_mention": true, "agent": "default", "media_dir": "~/.picobot/media/feishu", "reaction_emoji": "Typing" diff --git a/resources/skills/about-picobot/references/config.md b/resources/skills/about-picobot/references/config.md index 8da7608..2f45ca3 100644 --- a/resources/skills/about-picobot/references/config.md +++ b/resources/skills/about-picobot/references/config.md @@ -94,6 +94,7 @@ Gateway WebUI 的“配置”页可以编辑实际加载的配置文件。读取 | `app_id` | string | - | 飞书应用 ID | | `app_secret` | string | - | 飞书应用密钥 | | `allow_from` | []string | ["*"] | 允许交互的用户列表 | +| `require_mention` | bool | true | 群聊中是否必须明确 @ 机器人；无法解析机器人身份时安全地忽略群消息 | | `agent` | string | - | 使用的 agent 名称 | | `media_dir` | string | ~/.picobot/media/feishu | 配置默认值；Gateway 注册渠道时会覆盖为 `{workspace}/media/feishu` | | `reaction_emoji` | string | "Typing" | 回复意向表达的表情 | diff --git a/resources/templates/config.example.json b/resources/templates/config.example.json index bedc9ad..4b744e9 100644 --- a/resources/templates/config.example.json +++ b/resources/templates/config.example.json @@ -68,6 +68,7 @@ "app_id": "", "app_secret": "", "allow_from": ["*"], + "require_mention": true, "agent": "default", "media_dir": "~/.picobot/media/feishu", "reaction_emoji": "Typing", diff --git a/src/channels/feishu.rs b/src/channels/feishu.rs index a549a64..56dd706 100644 --- a/src/channels/feishu.rs +++ b/src/channels/feishu.rs @@ -130,11 +130,15 @@ struct LarkSenderId { struct LarkMessage { message_id: String, chat_id: String, + #[serde(default)] + chat_type: String, message_type: String, #[serde(default)] content: String, #[serde(default)] parent_id: Option, + #[serde(default)] + mentions: Vec, } // ───────────────────────────────────────────────────────────────────────────── @@ -158,6 +162,8 @@ pub struct FeishuChannel { tenant_token: Arc>>, /// Dedup cache: WS message_ids seen in the last ~30 min. seen_message_ids: Arc>>, + /// Bot identity used to enforce group-chat @mention admission. + bot_open_id: Arc>>, } /// Parsed message data from a Feishu frame @@ -190,9 +196,45 @@ impl FeishuChannel { connected: Arc::new(RwLock::new(false)), tenant_token: Arc::new(RwLock::new(None)), seen_message_ids: Arc::new(RwLock::new(HashMap::new())), + bot_open_id: Arc::new(RwLock::new(None)), }) } + async fn refresh_bot_open_id(&self) -> Result { + let token = self.get_tenant_access_token().await?; + let response = self + .http_client + .get(format!("{}/bot/v3/info", FEISHU_API_BASE)) + .bearer_auth(token) + .send() + .await + .map_err(|error| { + ChannelError::ConnectionError(format!("Bot info HTTP error: {error}")) + })?; + let status = response.status(); + let body: serde_json::Value = response.json().await.map_err(|error| { + ChannelError::Other(format!("Failed to parse bot info response: {error}")) + })?; + if !status.is_success() || body.get("code").and_then(|value| value.as_i64()) != Some(0) { + return Err(ChannelError::Other(format!( + "Bot info request failed: status={status}, code={}", + body.get("code") + .and_then(|value| value.as_i64()) + .unwrap_or(-1) + ))); + } + let open_id = body + .pointer("/bot/open_id") + .or_else(|| body.pointer("/data/bot/open_id")) + .and_then(|value| value.as_str()) + .map(str::trim) + .filter(|value| !value.is_empty()) + .ok_or_else(|| ChannelError::Other("Bot info response has no open_id".to_string()))? + .to_string(); + *self.bot_open_id.write().await = Some(open_id.clone()); + Ok(open_id) + } + /// Get WebSocket endpoint URL from Feishu API async fn get_ws_endpoint( &self, @@ -965,14 +1007,34 @@ impl FeishuChannel { let payload_data: MsgReceivePayload = serde_json::from_value(event.event.clone()) .map_err(|e| ChannelError::Other(format!("Parse payload error: {}", e)))?; - // Skip bot messages - if payload_data.sender.sender_type == "bot" { + // Never let bot/app traffic trigger another model turn. + if matches!(payload_data.sender.sender_type.as_str(), "bot" | "app") { + return Ok(None); + } + + let open_id = payload_data + .sender + .sender_id + .open_id + .ok_or_else(|| ChannelError::Other("No open_id".to_string()))?; + + if !self.is_allowed(&open_id) { + tracing::warn!(sender = %open_id, "Rejected unauthorized Feishu sender"); return Ok(None); } let message_id = payload_data.message.message_id.clone(); + let msg = payload_data.message; + if msg.chat_type == "group" && self.config.require_mention { + let bot_open_id = self.bot_open_id.read().await.clone(); + if !message_mentions_bot(&msg, bot_open_id.as_deref()) { + #[cfg(debug_assertions)] + tracing::debug!(message_id = %message_id, "Ignoring group message without bot mention"); + return Ok(None); + } + } - // Deduplication check + // Deduplicate only after admission so rejected traffic does not consume cache capacity. if self.is_message_seen(&message_id).await { #[cfg(debug_assertions)] tracing::debug!(message_id = %message_id, "Duplicate message, skipping"); @@ -982,13 +1044,6 @@ impl FeishuChannel { #[cfg(debug_assertions)] tracing::debug!(message_id = %message_id, "Received Feishu message"); - let open_id = payload_data - .sender - .sender_id - .open_id - .ok_or_else(|| ChannelError::Other("No open_id".to_string()))?; - - let msg = payload_data.message; let chat_id = msg.chat_id.clone(); let msg_type = msg.message_type.as_str(); let raw_content = msg.content.clone(); @@ -1000,6 +1055,11 @@ impl FeishuChannel { let (mut content, media) = self .parse_and_download_message(msg_type, &raw_content, &message_id) .await?; + content = normalize_mentions( + &content, + &msg.mentions, + self.bot_open_id.read().await.as_deref(), + ); // Fetch and prepend quoted message content if this is a reply if let Some(ref pid) = parent_id @@ -1139,9 +1199,7 @@ impl FeishuChannel { _ => (content.to_string(), Vec::new()), }; - // Strip @_user_N placeholders from group chat @mentions - let clean_text = strip_at_placeholders(&text); - Ok((clean_text, media)) + Ok((text, media)) } /// Send acknowledgment for a message @@ -1726,34 +1784,74 @@ fn extract_inline_text(el: &serde_json::Value, out: &mut String) { } } -/// Remove @_user_N placeholder tokens injected by Feishu in group chats -fn strip_at_placeholders(text: &str) -> String { - let mut result = String::with_capacity(text.len()); - let mut chars = text.chars().peekable(); +fn mention_open_id(mention: &serde_json::Value) -> Option<&str> { + mention + .pointer("/id/open_id") + .or_else(|| mention.get("open_id")) + .and_then(|value| value.as_str()) +} - while let Some(ch) = chars.next() { - if ch == '@' { - let rest: String = chars.clone().collect(); - if let Some(after) = rest.strip_prefix("_user_") { - // Skip until we hit a non-alphanumeric character - let placeholder_len = after - .find(|c: char| !c.is_alphanumeric()) - .unwrap_or(after.len()); - // Skip the placeholder - for _ in 0..placeholder_len { - chars.next(); - } - // Also skip the underscore after user_N if present - if chars.peek() == Some(&'_') { - chars.next(); - } - continue; - } - } - result.push(ch); +fn message_mentions_bot(message: &LarkMessage, bot_open_id: Option<&str>) -> bool { + let Some(bot_open_id) = bot_open_id.filter(|value| !value.is_empty()) else { + return false; + }; + if message + .mentions + .iter() + .any(|mention| mention_open_id(mention) == Some(bot_open_id)) + { + return true; } - result + fn post_contains_bot(value: &serde_json::Value, bot_open_id: &str) -> bool { + match value { + serde_json::Value::Object(map) => { + let is_bot_mention = map.get("tag").and_then(|value| value.as_str()) == Some("at") + && map + .get("user_id") + .or_else(|| map.get("open_id")) + .and_then(|value| value.as_str()) + == Some(bot_open_id); + is_bot_mention + || map + .values() + .any(|value| post_contains_bot(value, bot_open_id)) + } + serde_json::Value::Array(values) => values + .iter() + .any(|value| post_contains_bot(value, bot_open_id)), + _ => false, + } + } + + serde_json::from_str::(&message.content) + .is_ok_and(|content| post_contains_bot(&content, bot_open_id)) +} + +/// Remove the bot's own placeholder while preserving human mentions as readable names. +fn normalize_mentions( + text: &str, + mentions: &[serde_json::Value], + bot_open_id: Option<&str>, +) -> String { + let mut normalized = text.to_string(); + for mention in mentions { + let Some(key) = mention.get("key").and_then(|value| value.as_str()) else { + continue; + }; + let replacement = if mention_open_id(mention) == bot_open_id { + String::new() + } else { + mention + .get("name") + .and_then(|value| value.as_str()) + .filter(|value| !value.is_empty()) + .map(|name| format!("@{name}")) + .unwrap_or_else(|| key.to_string()) + }; + normalized = normalized.replace(key, &replacement); + } + normalized.trim().to_string() } fn resolve_image_ext(content_type: &str) -> &str { @@ -2165,6 +2263,13 @@ impl Channel for FeishuChannel { "feishu" } + fn is_allowed(&self, sender_id: &str) -> bool { + self.config.allow_from.iter().any(|allowed| { + let allowed = allowed.trim(); + allowed == "*" || allowed == sender_id + }) + } + /// Handle an inbound message: check for slash commands first, then publish to bus async fn handle_and_publish( &self, @@ -2184,6 +2289,18 @@ impl Channel for FeishuChannel { )); } + if self.config.require_mention { + match self.refresh_bot_open_id().await { + Ok(open_id) => { + tracing::info!(bot_open_id = %open_id, "Resolved Feishu bot identity") + } + Err(error) => tracing::warn!( + error = %error, + "Failed to resolve Feishu bot identity; group messages will be ignored" + ), + } + } + let mut run_task = self.run_task.lock().await; if run_task.as_ref().is_some_and(|task| !task.is_finished()) { return Ok(()); @@ -2563,6 +2680,7 @@ mod tests { app_id: "test-app".to_string(), app_secret: "test-secret".to_string(), allow_from: vec!["*".to_string()], + require_mention: true, agent: String::new(), media_dir: String::new(), reaction_emoji: "THUMBSUP".to_string(), @@ -2574,6 +2692,113 @@ mod tests { .expect("test channel should be valid") } + fn inbound_frame( + message_id: &str, + sender_id: &str, + chat_type: &str, + text: &str, + mentions: serde_json::Value, + ) -> PbFrame { + PbFrame { + seq_id: 1, + log_id: 1, + service: 1, + method: 1, + headers: vec![], + payload: Some( + serde_json::json!({ + "header": { + "event_type": "im.message.receive_v1", + "event_id": format!("event-{message_id}") + }, + "event": { + "sender": { + "sender_id": { "open_id": sender_id }, + "sender_type": "user" + }, + "message": { + "message_id": message_id, + "chat_id": "oc_test", + "chat_type": chat_type, + "message_type": "text", + "content": serde_json::json!({ "text": text }).to_string(), + "mentions": mentions + } + } + }) + .to_string() + .into_bytes(), + ), + } + } + + #[tokio::test] + async fn inbound_admission_enforces_allowlist_before_parsing() { + let mut channel = test_channel(); + channel.config.allow_from = vec!["ou_allowed".to_string()]; + let frame = inbound_frame( + "om_denied", + "ou_denied", + "p2p", + "hello", + serde_json::json!([]), + ); + + assert!(channel.handle_frame(&frame).await.unwrap().is_none()); + assert!(channel.seen_message_ids.read().await.is_empty()); + } + + #[tokio::test] + async fn group_message_requires_bot_mention_and_removes_only_self_mention() { + let channel = test_channel(); + *channel.bot_open_id.write().await = Some("ou_bot".to_string()); + + let ignored = inbound_frame( + "om_ignored", + "ou_user", + "group", + "hello", + serde_json::json!([]), + ); + assert!(channel.handle_frame(&ignored).await.unwrap().is_none()); + + let admitted = inbound_frame( + "om_admitted", + "ou_user", + "group", + "@_user_1 ask @_user_2", + serde_json::json!([ + {"key": "@_user_1", "id": {"open_id": "ou_bot"}, "name": "PicoBot"}, + {"key": "@_user_2", "id": {"open_id": "ou_peer"}, "name": "Alice"} + ]), + ); + let parsed = channel + .handle_frame(&admitted) + .await + .unwrap() + .expect("mentioned group message should be admitted"); + assert_eq!(parsed.content, "ask @Alice"); + } + + #[test] + fn post_mentions_can_gate_group_messages_when_top_level_mentions_are_absent() { + let message = LarkMessage { + message_id: "om_post".to_string(), + chat_id: "oc_test".to_string(), + chat_type: "group".to_string(), + message_type: "post".to_string(), + content: serde_json::json!({ + "zh_cn": {"content": [[{"tag": "at", "user_id": "ou_bot"}]]} + }) + .to_string(), + parent_id: None, + mentions: vec![], + }; + + assert!(message_mentions_bot(&message, Some("ou_bot"))); + assert!(!message_mentions_bot(&message, Some("ou_other"))); + } + #[tokio::test] async fn turn_sink_creates_once_updates_same_card_and_cleans_up_at_finish() { let state = Arc::new(Mutex::new(MockTurnState::default())); diff --git a/src/config/mod.rs b/src/config/mod.rs index b19c188..4397ab4 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -71,6 +71,9 @@ pub struct FeishuChannelConfig { pub app_secret: String, #[serde(default = "default_allow_from")] pub allow_from: Vec, + /// Require an explicit bot @mention before accepting group-chat messages. + #[serde(default = "default_true")] + pub require_mention: bool, #[serde(default)] pub agent: String, #[serde(default = "default_media_dir")]