fix: enforce telegram reaction authorization

2026-02-26 01:02:36 +01:00
parent c6dfa26f03
commit e56b0cf1a0
4 changed files with 260 additions and 54 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai

 - Slack/Session threads: prevent oversized parent-session inheritance from silently bricking new thread sessions, surface embedded context-overflow empty-result failures to users, and add configurable `session.parentForkMaxTokens` (default `100000`, `0` disables). (#26912) Thanks @markshields-tl.
 - Security/Signal: enforce DM/group authorization before reaction-only notification enqueue so unauthorized senders can no longer inject Signal reaction system events under `dmPolicy`/`groupPolicy`; reaction notifications now require channel access checks first. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
+- Security/Telegram reactions: enforce `dmPolicy`/`allowFrom` and group allowlist authorization on `message_reaction` events before enqueueing reaction system events, preventing unauthorized reaction-triggered input in DMs and groups; ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
 - Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
 - Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
 - Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
--- a/docs/channels/telegram.md
+++ b/docs/channels/telegram.md
@@ -553,6 +553,7 @@ curl "https://api.telegram.org/bot<bot_token>/getUpdates"
    Notes:

    - `own` means user reactions to bot-sent messages only (best-effort via sent-message cache).
+    - Reaction events still respect Telegram access controls (`dmPolicy`, `allowFrom`, `groupPolicy`, `groupAllowFrom`); unauthorized senders are dropped.
    - Telegram does not provide thread IDs in reaction updates.
      - non-forum groups route to group chat session
      - forum groups route to the group general-topic session (`:topic:1`), not the exact originating topic
--- a/src/telegram/bot-handlers.ts
+++ b/src/telegram/bot-handlers.ts
@@ -507,6 +507,99 @@ export const registerTelegramHandlers = ({
    return false;
  };

+  const isTelegramEventSenderAuthorized = async (params: {
+    chatId: number;
+    chatTitle?: string;
+    isGroup: boolean;
+    isForum: boolean;
+    messageThreadId?: number;
+    senderId: string;
+    senderUsername: string;
+    enforceDirectAuthorization: boolean;
+    enforceGroupAllowlistAuthorization: boolean;
+    deniedDmReason: string;
+    deniedGroupReason: string;
+    groupAllowContext?: Awaited<ReturnType<typeof resolveTelegramGroupAllowFromContext>>;
+  }) => {
+    const {
+      chatId,
+      chatTitle,
+      isGroup,
+      isForum,
+      messageThreadId,
+      senderId,
+      senderUsername,
+      enforceDirectAuthorization,
+      enforceGroupAllowlistAuthorization,
+      deniedDmReason,
+      deniedGroupReason,
+      groupAllowContext: preResolvedGroupAllowContext,
+    } = params;
+    const dmPolicy = telegramCfg.dmPolicy ?? "pairing";
+    const groupAllowContext =
+      preResolvedGroupAllowContext ??
+      (await resolveTelegramGroupAllowFromContext({
+        chatId,
+        accountId,
+        dmPolicy,
+        isForum,
+        messageThreadId,
+        groupAllowFrom,
+        resolveTelegramGroupConfig,
+      }));
+    const {
+      resolvedThreadId,
+      storeAllowFrom,
+      groupConfig,
+      topicConfig,
+      effectiveGroupAllow,
+      hasGroupAllowOverride,
+    } = groupAllowContext;
+    if (
+      shouldSkipGroupMessage({
+        isGroup,
+        chatId,
+        chatTitle,
+        resolvedThreadId,
+        senderId,
+        senderUsername,
+        effectiveGroupAllow,
+        hasGroupAllowOverride,
+        groupConfig,
+        topicConfig,
+      })
+    ) {
+      return false;
+    }
+
+    if (!isGroup && enforceDirectAuthorization) {
+      if (dmPolicy === "disabled") {
+        logVerbose(
+          `Blocked telegram direct event from ${senderId || "unknown"} (${deniedDmReason})`,
+        );
+        return false;
+      }
+      if (dmPolicy !== "open") {
+        const effectiveDmAllow = normalizeAllowFromWithStore({
+          allowFrom,
+          storeAllowFrom,
+          dmPolicy,
+        });
+        if (!isAllowlistAuthorized(effectiveDmAllow, senderId, senderUsername)) {
+          logVerbose(`Blocked telegram direct sender ${senderId || "unknown"} (${deniedDmReason})`);
+          return false;
+        }
+      }
+    }
+    if (isGroup && enforceGroupAllowlistAuthorization) {
+      if (!isAllowlistAuthorized(effectiveGroupAllow, senderId, senderUsername)) {
+        logVerbose(`Blocked telegram group sender ${senderId || "unknown"} (${deniedGroupReason})`);
+        return false;
+      }
+    }
+    return true;
+  };
+
  // Handle emoji reactions to messages.
  bot.on("message_reaction", async (ctx) => {
    try {
@@ -521,6 +614,10 @@ export const registerTelegramHandlers = ({
      const chatId = reaction.chat.id;
      const messageId = reaction.message_id;
      const user = reaction.user;
+      const senderId = user?.id != null ? String(user.id) : "";
+      const senderUsername = user?.username ?? "";
+      const isGroup = reaction.chat.type === "group" || reaction.chat.type === "supergroup";
+      const isForum = reaction.chat.is_forum === true;

      // Resolve reaction notification mode (default: "own").
      const reactionMode = telegramCfg.reactionNotifications ?? "own";
@@ -533,6 +630,21 @@ export const registerTelegramHandlers = ({
      if (reactionMode === "own" && !wasSentByBot(chatId, messageId)) {
        return;
      }
+      const senderAuthorized = await isTelegramEventSenderAuthorized({
+        chatId,
+        chatTitle: reaction.chat.title,
+        isGroup,
+        isForum,
+        senderId,
+        senderUsername,
+        enforceDirectAuthorization: true,
+        enforceGroupAllowlistAuthorization: false,
+        deniedDmReason: "reaction unauthorized by dm policy/allowlist",
+        deniedGroupReason: "reaction unauthorized by group allowlist",
+      });
+      if (!senderAuthorized) {
+        return;
+      }

      // Detect added reactions.
      const oldEmojis = new Set(
@@ -552,12 +664,12 @@ export const registerTelegramHandlers = ({
      const senderName = user
        ? [user.first_name, user.last_name].filter(Boolean).join(" ").trim() || user.username
        : undefined;
-      const senderUsername = user?.username ? `@${user.username}` : undefined;
+      const senderUsernameLabel = user?.username ? `@${user.username}` : undefined;
      let senderLabel = senderName;
-      if (senderName && senderUsername) {
-        senderLabel = `${senderName} (${senderUsername})`;
-      } else if (!senderName && senderUsername) {
-        senderLabel = senderUsername;
+      if (senderName && senderUsernameLabel) {
+        senderLabel = `${senderName} (${senderUsernameLabel})`;
+      } else if (!senderName && senderUsernameLabel) {
+        senderLabel = senderUsernameLabel;
      }
      if (!senderLabel && user?.id) {
        senderLabel = `id:${user.id}`;
@@ -567,8 +679,6 @@ export const registerTelegramHandlers = ({
      // Reactions target a specific message_id; the Telegram Bot API does not include
      // message_thread_id on MessageReactionUpdated, so we route to the chat-level
      // session (forum topic routing is not available for reactions).
-      const isGroup = reaction.chat.type === "group" || reaction.chat.type === "supergroup";
-      const isForum = reaction.chat.is_forum === true;
      const resolvedThreadId = isForum
        ? resolveTelegramForumThreadId({ isForum, messageThreadId: undefined })
        : undefined;
@@ -864,58 +974,27 @@ export const registerTelegramHandlers = ({
        groupAllowFrom,
        resolveTelegramGroupConfig,
      });
-      const {
-        resolvedThreadId,
-        storeAllowFrom,
-        groupConfig,
-        topicConfig,
-        effectiveGroupAllow,
-        hasGroupAllowOverride,
-      } = groupAllowContext;
-      const dmPolicy = telegramCfg.dmPolicy ?? "pairing";
-      const effectiveDmAllow = normalizeAllowFromWithStore({
-        allowFrom: telegramCfg.allowFrom,
-        storeAllowFrom,
-        dmPolicy,
-      });
+      const { resolvedThreadId, storeAllowFrom } = groupAllowContext;
      const senderId = callback.from?.id ? String(callback.from.id) : "";
      const senderUsername = callback.from?.username ?? "";
-      if (
-        shouldSkipGroupMessage({
-          isGroup,
-          chatId,
-          chatTitle: callbackMessage.chat.title,
-          resolvedThreadId,
-          senderId,
-          senderUsername,
-          effectiveGroupAllow,
-          hasGroupAllowOverride,
-          groupConfig,
-          topicConfig,
-        })
-      ) {
+      const senderAuthorized = await isTelegramEventSenderAuthorized({
+        chatId,
+        chatTitle: callbackMessage.chat.title,
+        isGroup,
+        isForum,
+        messageThreadId,
+        senderId,
+        senderUsername,
+        enforceDirectAuthorization: inlineButtonsScope === "allowlist",
+        enforceGroupAllowlistAuthorization: inlineButtonsScope === "allowlist",
+        deniedDmReason: "callback unauthorized by inlineButtonsScope allowlist",
+        deniedGroupReason: "callback unauthorized by inlineButtonsScope allowlist",
+        groupAllowContext,
+      });
+      if (!senderAuthorized) {
        return;
      }

-      if (inlineButtonsScope === "allowlist") {
-        if (!isGroup) {
-          if (dmPolicy === "disabled") {
-            return;
-          }
-          if (dmPolicy !== "open") {
-            const allowed = isAllowlistAuthorized(effectiveDmAllow, senderId, senderUsername);
-            if (!allowed) {
-              return;
-            }
-          }
-        } else {
-          const allowed = isAllowlistAuthorized(effectiveGroupAllow, senderId, senderUsername);
-          if (!allowed) {
-            return;
-          }
-        }
-      }
-
      const paginationMatch = data.match(/^commands_page_(\d+|noop)(?::(.+))?$/);
      if (paginationMatch) {
        const pageValue = paginationMatch[1];
--- a/src/telegram/bot.test.ts
+++ b/src/telegram/bot.test.ts
@@ -832,6 +832,131 @@ describe("createTelegramBot", () => {
    );
  });

+  it("blocks reaction when dmPolicy is disabled", async () => {
+    onSpy.mockClear();
+    enqueueSystemEventSpy.mockClear();
+
+    loadConfig.mockReturnValue({
+      channels: {
+        telegram: { dmPolicy: "disabled", reactionNotifications: "all" },
+      },
+    });
+
+    createTelegramBot({ token: "tok" });
+    const handler = getOnHandler("message_reaction") as (
+      ctx: Record<string, unknown>,
+    ) => Promise<void>;
+
+    await handler({
+      update: { update_id: 510 },
+      messageReaction: {
+        chat: { id: 1234, type: "private" },
+        message_id: 42,
+        user: { id: 9, first_name: "Ada" },
+        date: 1736380800,
+        old_reaction: [],
+        new_reaction: [{ type: "emoji", emoji: "👍" }],
+      },
+    });
+
+    expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
+  });
+
+  it("blocks reaction in allowlist mode for unauthorized direct sender", async () => {
+    onSpy.mockClear();
+    enqueueSystemEventSpy.mockClear();
+
+    loadConfig.mockReturnValue({
+      channels: {
+        telegram: { dmPolicy: "allowlist", allowFrom: ["12345"], reactionNotifications: "all" },
+      },
+    });
+
+    createTelegramBot({ token: "tok" });
+    const handler = getOnHandler("message_reaction") as (
+      ctx: Record<string, unknown>,
+    ) => Promise<void>;
+
+    await handler({
+      update: { update_id: 511 },
+      messageReaction: {
+        chat: { id: 1234, type: "private" },
+        message_id: 42,
+        user: { id: 9, first_name: "Ada" },
+        date: 1736380800,
+        old_reaction: [],
+        new_reaction: [{ type: "emoji", emoji: "👍" }],
+      },
+    });
+
+    expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
+  });
+
+  it("allows reaction in allowlist mode for authorized direct sender", async () => {
+    onSpy.mockClear();
+    enqueueSystemEventSpy.mockClear();
+
+    loadConfig.mockReturnValue({
+      channels: {
+        telegram: { dmPolicy: "allowlist", allowFrom: ["9"], reactionNotifications: "all" },
+      },
+    });
+
+    createTelegramBot({ token: "tok" });
+    const handler = getOnHandler("message_reaction") as (
+      ctx: Record<string, unknown>,
+    ) => Promise<void>;
+
+    await handler({
+      update: { update_id: 512 },
+      messageReaction: {
+        chat: { id: 1234, type: "private" },
+        message_id: 42,
+        user: { id: 9, first_name: "Ada" },
+        date: 1736380800,
+        old_reaction: [],
+        new_reaction: [{ type: "emoji", emoji: "👍" }],
+      },
+    });
+
+    expect(enqueueSystemEventSpy).toHaveBeenCalledTimes(1);
+  });
+
+  it("blocks reaction in group allowlist mode for unauthorized sender", async () => {
+    onSpy.mockClear();
+    enqueueSystemEventSpy.mockClear();
+
+    loadConfig.mockReturnValue({
+      channels: {
+        telegram: {
+          dmPolicy: "open",
+          groupPolicy: "allowlist",
+          groupAllowFrom: ["12345"],
+          reactionNotifications: "all",
+        },
+      },
+    });
+
+    createTelegramBot({ token: "tok" });
+    const handler = getOnHandler("message_reaction") as (
+      ctx: Record<string, unknown>,
+    ) => Promise<void>;
+
+    await handler({
+      update: { update_id: 513 },
+      messageReaction: {
+        chat: { id: 9999, type: "supergroup" },
+        message_id: 77,
+        user: { id: 9, first_name: "Ada" },
+        date: 1736380800,
+        old_reaction: [],
+        new_reaction: [{ type: "emoji", emoji: "🔥" }],
+      },
+    });
+
+    expect(enqueueSystemEventSpy).not.toHaveBeenCalled();
+  });
+
  it("skips reaction when reactionNotifications is off", async () => {
    onSpy.mockClear();
    enqueueSystemEventSpy.mockClear();