mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(telegram): deny all users when allowed_users has no valid ID (#6230)
* fix(telegram): deny all users when allowed_users has no valid ID
TelegramChannel parsed channels.telegram.allowed_users with int() and
dropped every entry that failed without logging, and _check_user treated
an empty result as "no allowlist". A list of @usernames therefore opened
the bot to everyone. A single string ID was iterated character by
character ("123456" allowed users 1-6 and blocked 123456), which is what
an allowed_users: $ENV reference resolves to, and null or a bare integer
crashed the channel at startup.
The allowlist is now parsed into frozenset[int] | None: None (unset,
null, empty list, blank string) still allows everyone, a single ID is a
one-entry list, entries that are not positive numeric IDs (@usernames,
floats, booleans, comma-joined strings) are dropped with a warning, and
a configured list with no valid ID denies every user and logs an error.
* docs(changelog): reference #6230 for the telegram allowed_users fix
* fix(telegram): say "positive" in the allowed_users drop warning
0 and negative integers are numeric but still dropped, so "expected one
numeric Telegram user ID" misdescribed them. The warning now asks for a
positive numeric ID, matching _parse_telegram_user_id, and the drop test
covers 0 and -5 alongside @alice.
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
This commit is contained in:
parent
beaae22088
commit
9bcf93ec40
10
CHANGELOG.md
10
CHANGELOG.md
@ -2790,6 +2790,15 @@ This release closes that milestone with **301 merged pull requests**.
|
||||
still miss. Argument values that spell a denied path in order are refused too
|
||||
(fail-closed). ([#6212])
|
||||
|
||||
- **channels:** A Telegram `allowed_users` list that contains no numeric user ID
|
||||
now denies every user instead of silently allowing all of them. Entries that
|
||||
failed `int()` were dropped without a log line, and an empty result meant "no
|
||||
allowlist", so `["@alice", "bob"]` opened the bot to everyone. A single ID is
|
||||
now a one-entry list rather than a string whose digits each became an allowed
|
||||
user (`"123456"` allowed users 1–6 and blocked 123456), `null` or a bare
|
||||
integer no longer crashes the channel at startup, and every dropped entry —
|
||||
`@usernames`, floats, booleans — is logged as a warning. ([#6230])
|
||||
|
||||
### Documentation
|
||||
|
||||
- **docs:** Fix the Apple Container verification instructions. The guide
|
||||
@ -7662,3 +7671,4 @@ with **180 merged pull requests** since the first 2.0 milestone tag.
|
||||
[#6202]: https://github.com/bytedance/deer-flow/pull/6202
|
||||
[#6214]: https://github.com/bytedance/deer-flow/pull/6214
|
||||
[#6212]: https://github.com/bytedance/deer-flow/pull/6212
|
||||
[#6230]: https://github.com/bytedance/deer-flow/pull/6230
|
||||
|
||||
@ -2360,6 +2360,13 @@
|
||||
(`config --profile work show`),而连续匹配仍会漏掉这种情况。参数值恰好按
|
||||
顺序拼出被拒绝路径的调用也会被拒绝(fail-closed)。([#6212])
|
||||
|
||||
- **渠道:** Telegram 的 `allowed_users` 列表中若没有任何数字用户 ID,现在会拒绝
|
||||
所有用户,而不是静默放行所有人。此前无法通过 `int()` 的条目会被直接丢弃且不
|
||||
记录日志,而结果为空又被视为"未配置白名单",因此 `["@alice", "bob"]` 会让机器
|
||||
人对所有人开放。单个 ID 现在视为只有一项的列表,而不再被当作字符串逐位拆成多个
|
||||
用户(`"123456"` 曾放行用户 1–6 并拦截 123456);`null` 或单个整数也不再导致
|
||||
渠道启动时崩溃;每个被丢弃的条目(`@用户名`、浮点数、布尔值)都会记录警告。([#6230])
|
||||
|
||||
### 文档
|
||||
|
||||
- **文档:** 修正 Apple Container 的验证说明。
|
||||
@ -6401,3 +6408,4 @@ DeerFlow 2.0 是围绕"超级智能体"框架的彻底重写,核心包含子
|
||||
[#6202]: https://github.com/bytedance/deer-flow/pull/6202
|
||||
[#6212]: https://github.com/bytedance/deer-flow/pull/6212
|
||||
[#6214]: https://github.com/bytedance/deer-flow/pull/6214
|
||||
[#6230]: https://github.com/bytedance/deer-flow/pull/6230
|
||||
|
||||
@ -801,7 +801,7 @@ channels:
|
||||
bot_token: $TELEGRAM_BOT_TOKEN
|
||||
# Optional: render final Markdown replies as Telegram Rich Messages.
|
||||
rich_messages: false
|
||||
allowed_users: [] # empty = allow all
|
||||
allowed_users: [] # numeric user IDs, not @usernames; empty = allow all
|
||||
|
||||
wechat:
|
||||
enabled: false
|
||||
|
||||
@ -349,7 +349,7 @@ channels:
|
||||
telegram:
|
||||
enabled: true
|
||||
bot_token: $TELEGRAM_BOT_TOKEN
|
||||
allowed_users: [] # empty = allow all
|
||||
allowed_users: [] # identifiants numériques, pas de @pseudo ; vide = tout le monde autorisé
|
||||
|
||||
# Optional: per-channel / per-user session settings
|
||||
session:
|
||||
|
||||
@ -336,7 +336,7 @@ channels:
|
||||
telegram:
|
||||
enabled: true
|
||||
bot_token: $TELEGRAM_BOT_TOKEN
|
||||
allowed_users: [] # 空 = 全員許可
|
||||
allowed_users: [] # 数値のユーザー ID(@ユーザー名は不可)、空 = 全員許可
|
||||
|
||||
# オプション: チャネル/ユーザーごとのセッション設定
|
||||
session:
|
||||
|
||||
@ -323,7 +323,7 @@ channels:
|
||||
telegram:
|
||||
enabled: true
|
||||
bot_token: $TELEGRAM_BOT_TOKEN
|
||||
allowed_users: []
|
||||
allowed_users: [] # числовые ID пользователей, не @username; пусто = разрешить всем
|
||||
|
||||
wechat:
|
||||
enabled: false
|
||||
|
||||
@ -445,7 +445,7 @@ channels:
|
||||
telegram:
|
||||
enabled: true
|
||||
bot_token: $TELEGRAM_BOT_TOKEN
|
||||
allowed_users: [] # 留空表示允许所有人
|
||||
allowed_users: [] # 填数字用户 ID,不是 @用户名;留空表示允许所有人
|
||||
|
||||
# 可选:按渠道 / 按用户单独覆盖 session 配置
|
||||
session:
|
||||
|
||||
@ -52,7 +52,7 @@ The cached value is reused for both the blocking (`runs.wait`) and streaming (`_
|
||||
- `langgraph_url` - LangGraph-compatible Gateway API base URL (default: `http://localhost:8001/api`)
|
||||
- `gateway_url` - Gateway API URL for auxiliary commands (default: `http://localhost:8001`)
|
||||
- In Docker Compose, IM channels run inside the `gateway` container, so `localhost` points back to that container. Use `http://gateway:8001/api` for `langgraph_url` and `http://gateway:8001` for `gateway_url`, or set `DEER_FLOW_CHANNELS_LANGGRAPH_URL` / `DEER_FLOW_CHANNELS_GATEWAY_URL`.
|
||||
- Per-channel configs: `feishu` (app_id, app_secret), `slack` (bot_token, app_token), `telegram` (bot_token, optional `rich_messages` for final Markdown Rich Messages), `dingtalk` (client_id, client_secret, optional `card_template_id` for AI Card streaming), `github` (operator kill-switch `enabled`, plus `default_mention_login` for mention-required GitHub triggers), `buzz` (relay_url, private_key)
|
||||
- Per-channel configs: `feishu` (app_id, app_secret), `slack` (bot_token, app_token), `telegram` (bot_token, optional `rich_messages` for final Markdown Rich Messages; `allowed_users` takes numeric IDs or one ID, warns on unparseable entries like `@names`, and denies everyone if none parse), `dingtalk` (client_id, client_secret, optional `card_template_id` for AI Card streaming), `github` (operator kill-switch `enabled`, plus `default_mention_login` for mention-required GitHub triggers), `buzz` (relay_url, private_key)
|
||||
|
||||
**User-owned channel connections** (`config.yaml` -> `channel_connections`):
|
||||
- Disabled by default. It is a user-binding layer on top of the existing `channels.*` runtime config, not a replacement for provider bot credentials.
|
||||
|
||||
@ -98,12 +98,53 @@ def _load_telegram_input_file(path, filename: str):
|
||||
return InputFile(path.read_bytes(), filename=filename)
|
||||
|
||||
|
||||
def _parse_telegram_user_id(entry: Any) -> int | None:
|
||||
"""A positive Telegram user ID from an ``int`` or a digit string, else ``None``."""
|
||||
if isinstance(entry, bool):
|
||||
# bool is an int subclass: ``true`` must not become user 1.
|
||||
return None
|
||||
if isinstance(entry, int):
|
||||
return entry if entry > 0 else None
|
||||
if isinstance(entry, str):
|
||||
text = entry.strip()
|
||||
if text.isascii() and text.isdigit():
|
||||
return int(text) or None
|
||||
return None
|
||||
|
||||
|
||||
def _parse_allowed_users(allowed_users: Any) -> frozenset[int] | None:
|
||||
"""Parse ``channels.telegram.allowed_users``; ``None`` means no allowlist.
|
||||
|
||||
A single ID is shorthand for a one-entry list. Entries that are not numeric
|
||||
user IDs (``@usernames``, floats, booleans) are dropped with a warning. If
|
||||
none remain, the result is an empty set that denies everyone: the operator
|
||||
asked for a restriction, so an unreadable one must not open the bot to all.
|
||||
"""
|
||||
if allowed_users is None or (isinstance(allowed_users, str) and not allowed_users.strip()):
|
||||
return None
|
||||
entries = list(allowed_users) if isinstance(allowed_users, list | tuple | set) else [allowed_users]
|
||||
if not entries:
|
||||
return None
|
||||
user_ids: set[int] = set()
|
||||
for entry in entries:
|
||||
user_id = _parse_telegram_user_id(entry)
|
||||
if user_id is None:
|
||||
logger.warning("[Telegram] Ignoring allowed_users entry %r: expected a positive numeric Telegram user ID (not an @username); list several IDs as a YAML list", entry)
|
||||
else:
|
||||
user_ids.add(user_id)
|
||||
if not user_ids:
|
||||
logger.error("[Telegram] allowed_users has no valid numeric user ID; denying every user until it is fixed")
|
||||
return frozenset(user_ids)
|
||||
|
||||
|
||||
class TelegramChannel(Channel):
|
||||
"""Telegram bot channel using long-polling.
|
||||
|
||||
Configuration keys (in ``config.yaml`` under ``channels.telegram``):
|
||||
- ``bot_token``: Telegram Bot API token (from @BotFather).
|
||||
- ``allowed_users``: (optional) List of allowed Telegram user IDs. Empty = allow all.
|
||||
- ``allowed_users``: (optional) List of numeric Telegram user IDs (not
|
||||
@usernames), or a single ID. Empty = allow all; a non-empty list with
|
||||
no valid ID denies everyone.
|
||||
"""
|
||||
|
||||
def __init__(self, bus: MessageBus, config: dict[str, Any]) -> None:
|
||||
@ -123,12 +164,7 @@ class TelegramChannel(Channel):
|
||||
# Tasks submitted from the main dispatcher loop back to PTB's loop.
|
||||
# Only the Telegram loop mutates this set.
|
||||
self._tg_bridge_tasks: set[asyncio.Task[Any]] = set()
|
||||
self._allowed_users: set[int] = set()
|
||||
for uid in config.get("allowed_users", []):
|
||||
try:
|
||||
self._allowed_users.add(int(uid))
|
||||
except (ValueError, TypeError):
|
||||
pass
|
||||
self._allowed_users = _parse_allowed_users(config.get("allowed_users"))
|
||||
# chat_id -> last sent message_id for threaded replies
|
||||
self._last_bot_message: dict[str, int] = {}
|
||||
# stream_key ("chat_id:thread_ts") -> state of the in-flight streamed
|
||||
@ -916,7 +952,7 @@ class TelegramChannel(Channel):
|
||||
logger.exception("Error during Telegram shutdown")
|
||||
|
||||
def _check_user(self, user_id: int) -> bool:
|
||||
if not self._allowed_users:
|
||||
if self._allowed_users is None:
|
||||
return True
|
||||
return user_id in self._allowed_users
|
||||
|
||||
|
||||
@ -9167,6 +9167,85 @@ class TestSlackAllowedUsers:
|
||||
_run(go())
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Telegram allowed_users tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTelegramAllowedUsers:
|
||||
"""An allowlist the operator configured must never silently open the bot."""
|
||||
|
||||
@staticmethod
|
||||
def _channel(config_extra: dict):
|
||||
from app.channels.telegram import TelegramChannel
|
||||
|
||||
return TelegramChannel(bus=MessageBus(), config={"bot_token": "test-token", **config_extra})
|
||||
|
||||
@pytest.mark.parametrize("config_extra", [{}, {"allowed_users": None}, {"allowed_users": []}, {"allowed_users": " "}])
|
||||
def test_unset_or_empty_allowlist_allows_everyone_without_warning(self, config_extra, caplog):
|
||||
with caplog.at_level(logging.WARNING, logger="app.channels.telegram"):
|
||||
ch = self._channel(config_extra)
|
||||
|
||||
assert ch._check_user(42)
|
||||
assert caplog.records == []
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"allowed_users",
|
||||
[[123456, 7], ["123456", " 7 "], (123456, 7), {123456, 7}],
|
||||
)
|
||||
def test_numeric_ids_are_allowed_and_others_denied(self, allowed_users):
|
||||
ch = self._channel({"allowed_users": allowed_users})
|
||||
|
||||
assert ch._check_user(123456)
|
||||
assert ch._check_user(7)
|
||||
assert not ch._check_user(42)
|
||||
|
||||
@pytest.mark.parametrize("allowed_users", ["123456", 123456], ids=["str", "int"])
|
||||
def test_scalar_id_is_one_entry_not_its_digits(self, allowed_users):
|
||||
# Iterating the string "123456" used to allow users 1..6 and block 123456.
|
||||
ch = self._channel({"allowed_users": allowed_users})
|
||||
|
||||
assert ch._check_user(123456)
|
||||
assert not ch._check_user(1)
|
||||
|
||||
@pytest.mark.parametrize("bad_entry", ["@alice", 0, -5])
|
||||
def test_unparseable_entry_is_dropped_with_warning(self, bad_entry, caplog):
|
||||
with caplog.at_level(logging.WARNING, logger="app.channels.telegram"):
|
||||
ch = self._channel({"allowed_users": [123456, bad_entry]})
|
||||
|
||||
assert ch._check_user(123456)
|
||||
assert not ch._check_user(42)
|
||||
assert repr(bad_entry) in caplog.text
|
||||
# 0 and -5 are numeric, so the hint has to say what they are missing.
|
||||
assert "positive numeric" in caplog.text
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"allowed_users",
|
||||
# "123456,789" is what a ``$ENV`` reference to a comma-separated value resolves to.
|
||||
[["@alice", "bob"], "@alice", "123456,789", [True], [4.2], {"id": 42}],
|
||||
)
|
||||
def test_allowlist_without_a_parseable_entry_denies_everyone(self, allowed_users, caplog):
|
||||
with caplog.at_level(logging.WARNING, logger="app.channels.telegram"):
|
||||
ch = self._channel({"allowed_users": allowed_users})
|
||||
|
||||
assert not ch._check_user(42)
|
||||
assert not ch._check_user(1)
|
||||
assert any(record.levelno == logging.ERROR for record in caplog.records)
|
||||
|
||||
@pytest.mark.parametrize(("allowed_users", "admitted"), [(["@alice"], False), ([42], True)])
|
||||
def test_on_text_applies_the_allowlist(self, allowed_users, admitted):
|
||||
async def go():
|
||||
ch = self._channel({"allowed_users": allowed_users})
|
||||
ch._main_loop = asyncio.get_running_loop()
|
||||
ch._reserve_inbound = MagicMock(return_value=None)
|
||||
|
||||
await ch._on_text(_make_telegram_update("private", message_id=10), None)
|
||||
|
||||
assert ch._reserve_inbound.called is admitted
|
||||
|
||||
_run(go())
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Telegram send retry tests
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@ -2938,7 +2938,7 @@ run_ownership:
|
||||
# telegram:
|
||||
# enabled: false
|
||||
# bot_token: $TELEGRAM_BOT_TOKEN
|
||||
# allowed_users: [] # empty = allow all
|
||||
# allowed_users: [] # numeric user IDs, not @usernames; empty = allow all
|
||||
# rich_messages: false # Bot API 10.1 Rich Messages for final Markdown responses
|
||||
#
|
||||
# wechat:
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user