fix(signal): enforce mention gating for group messages (#13124)

* fix(signal): enforce mention gating for group messages Signal group messages bypassed mention gating, causing the bot to reply even when requireMention was enabled and the message did not mention the bot. This aligns Signal with Slack, Discord, Telegram, and iMessage which all enforce mention gating correctly. Fixes #13106 Co-Authored-By: Claude <noreply@anthropic.com> * fix(signal): keep pending history context for mention-gated skips (#13124) (thanks @zerone0x) --------- Co-authored-by: Yansu <no-reply@yansu.ai> Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
2026-02-19 18:39:20 -05:00 · 2026-02-10 13:19:07 +08:00
parent 137b7d9aab
commit 1d46ca3a95
2 changed files with 283 additions and 0 deletions
--- a/src/signal/monitor/event-handler.mention-gating.test.ts
+++ b/src/signal/monitor/event-handler.mention-gating.test.ts
@@ -0,0 +1,206 @@
+import { describe, expect, it, vi } from "vitest";
+import type { MsgContext } from "../../auto-reply/templating.js";
+
+let capturedCtx: MsgContext | undefined;
+
+vi.mock("../../auto-reply/dispatch.js", async (importOriginal) => {
+  const actual = await importOriginal<typeof import("../../auto-reply/dispatch.js")>();
+  const dispatchInboundMessage = vi.fn(async (params: { ctx: MsgContext }) => {
+    capturedCtx = params.ctx;
+    return { queuedFinal: false, counts: { tool: 0, block: 0, final: 0 } };
+  });
+  return {
+    ...actual,
+    dispatchInboundMessage,
+    dispatchInboundMessageWithDispatcher: dispatchInboundMessage,
+    dispatchInboundMessageWithBufferedDispatcher: dispatchInboundMessage,
+  };
+});
+
+import { createSignalEventHandler } from "./event-handler.js";
+
+function createBaseDeps(overrides: Record<string, unknown> = {}) {
+  return {
+    // oxlint-disable-next-line typescript/no-explicit-any
+    runtime: { log: () => {}, error: () => {} } as any,
+    baseUrl: "http://localhost",
+    accountId: "default",
+    historyLimit: 5,
+    groupHistories: new Map(),
+    textLimit: 4000,
+    dmPolicy: "open" as const,
+    allowFrom: ["*"],
+    groupAllowFrom: ["*"],
+    groupPolicy: "open" as const,
+    reactionMode: "off" as const,
+    reactionAllowlist: [],
+    mediaMaxBytes: 1024,
+    ignoreAttachments: true,
+    sendReadReceipts: false,
+    readReceiptsViaDaemon: false,
+    fetchAttachment: async () => null,
+    deliverReplies: async () => {},
+    resolveSignalReactionTargets: () => [],
+    // oxlint-disable-next-line typescript/no-explicit-any
+    isSignalReactionMessage: () => false as any,
+    shouldEmitSignalReactionNotification: () => false,
+    buildSignalReactionSystemEventText: () => "reaction",
+    ...overrides,
+  };
+}
+
+type GroupEventOpts = {
+  message?: string;
+  attachments?: unknown[];
+  quoteText?: string;
+};
+
+function makeGroupEvent(opts: GroupEventOpts) {
+  return {
+    event: "receive",
+    data: JSON.stringify({
+      envelope: {
+        sourceNumber: "+15550001111",
+        sourceName: "Alice",
+        timestamp: 1700000000000,
+        dataMessage: {
+          message: opts.message ?? "",
+          attachments: opts.attachments ?? [],
+          quote: opts.quoteText ? { text: opts.quoteText } : undefined,
+          groupInfo: { groupId: "g1", groupName: "Test Group" },
+        },
+      },
+    }),
+  };
+}
+
+describe("signal mention gating", () => {
+  it("drops group messages without mention when requireMention is configured", async () => {
+    capturedCtx = undefined;
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: true } } } },
+        },
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "hello everyone" }));
+    expect(capturedCtx).toBeUndefined();
+  });
+
+  it("allows group messages with mention when requireMention is configured", async () => {
+    capturedCtx = undefined;
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: true } } } },
+        },
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "hey @bot what's up" }));
+    expect(capturedCtx).toBeTruthy();
+    expect(capturedCtx?.WasMentioned).toBe(true);
+  });
+
+  it("sets WasMentioned=false for group messages without mention when requireMention is off", async () => {
+    capturedCtx = undefined;
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: false } } } },
+        },
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "hello everyone" }));
+    expect(capturedCtx).toBeTruthy();
+    expect(capturedCtx?.WasMentioned).toBe(false);
+  });
+
+  it("records pending history for skipped group messages", async () => {
+    capturedCtx = undefined;
+    const groupHistories = new Map();
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: true } } } },
+        },
+        historyLimit: 5,
+        groupHistories,
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "hello from alice" }));
+    expect(capturedCtx).toBeUndefined();
+    const entries = groupHistories.get("g1");
+    expect(entries).toBeTruthy();
+    expect(entries).toHaveLength(1);
+    expect(entries[0].sender).toBe("Alice");
+    expect(entries[0].body).toBe("hello from alice");
+  });
+
+  it("records attachment placeholder in pending history for skipped attachment-only group messages", async () => {
+    capturedCtx = undefined;
+    const groupHistories = new Map();
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: true } } } },
+        },
+        historyLimit: 5,
+        groupHistories,
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "", attachments: [{ id: "a1" }] }));
+    expect(capturedCtx).toBeUndefined();
+    const entries = groupHistories.get("g1");
+    expect(entries).toBeTruthy();
+    expect(entries).toHaveLength(1);
+    expect(entries[0].body).toBe("<media:attachment>");
+  });
+
+  it("records quote text in pending history for skipped quote-only group messages", async () => {
+    capturedCtx = undefined;
+    const groupHistories = new Map();
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: true } } } },
+        },
+        historyLimit: 5,
+        groupHistories,
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "", quoteText: "quoted context" }));
+    expect(capturedCtx).toBeUndefined();
+    const entries = groupHistories.get("g1");
+    expect(entries).toBeTruthy();
+    expect(entries).toHaveLength(1);
+    expect(entries[0].body).toBe("quoted context");
+  });
+
+  it("bypasses mention gating for authorized control commands", async () => {
+    capturedCtx = undefined;
+    const handler = createSignalEventHandler(
+      createBaseDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 }, groupChat: { mentionPatterns: ["@bot"] } },
+          channels: { signal: { groups: { "*": { requireMention: true } } } },
+        },
+      }),
+    );
+
+    await handler(makeGroupEvent({ message: "/help" }));
+    expect(capturedCtx).toBeTruthy();
+  });
+});
--- a/src/signal/monitor/event-handler.ts
+++ b/src/signal/monitor/event-handler.ts
@@ -14,14 +14,18 @@ import {
 import {
  buildPendingHistoryContextFromMap,
  clearHistoryEntriesIfEnabled,
+  recordPendingHistoryEntryIfEnabled,
 } from "../../auto-reply/reply/history.js";
 import { finalizeInboundContext } from "../../auto-reply/reply/inbound-context.js";
+import { buildMentionRegexes, matchesMentionPatterns } from "../../auto-reply/reply/mentions.js";
 import { createReplyDispatcherWithTyping } from "../../auto-reply/reply/reply-dispatcher.js";
 import { resolveControlCommandGate } from "../../channels/command-gating.js";
 import { logInboundDrop, logTypingFailure } from "../../channels/logging.js";
+import { resolveMentionGatingWithBypass } from "../../channels/mention-gating.js";
 import { createReplyPrefixOptions } from "../../channels/reply-prefix.js";
 import { recordInboundSession } from "../../channels/session.js";
 import { createTypingCallbacks } from "../../channels/typing.js";
+import { resolveChannelGroupRequireMention } from "../../config/group-policy.js";
 import { readSessionUpdatedAt, resolveStorePath } from "../../config/sessions.js";
 import { danger, logVerbose, shouldLogVerbose } from "../../globals.js";
 import { enqueueSystemEvent } from "../../infra/system-events.js";
@@ -61,6 +65,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
    mediaPath?: string;
    mediaType?: string;
    commandAuthorized: boolean;
+    wasMentioned?: boolean;
  };

  async function handleSignalInboundMessage(entry: SignalInboundEntry) {
@@ -144,6 +149,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
      MediaPath: entry.mediaPath,
      MediaType: entry.mediaType,
      MediaUrl: entry.mediaPath,
+      WasMentioned: entry.isGroup ? entry.wasMentioned === true : undefined,
      CommandAuthorized: entry.commandAuthorized,
      OriginatingChannel: "signal" as const,
      OriginatingTo: signalTo,
@@ -499,6 +505,76 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
      return;
    }

+    const route = resolveAgentRoute({
+      cfg: deps.cfg,
+      channel: "signal",
+      accountId: deps.accountId,
+      peer: {
+        kind: isGroup ? "group" : "direct",
+        id: isGroup ? (groupId ?? "unknown") : senderPeerId,
+      },
+    });
+    const mentionRegexes = buildMentionRegexes(deps.cfg, route.agentId);
+    const wasMentioned = isGroup && matchesMentionPatterns(messageText, mentionRegexes);
+    const requireMention =
+      isGroup &&
+      resolveChannelGroupRequireMention({
+        cfg: deps.cfg,
+        channel: "signal",
+        groupId,
+        accountId: deps.accountId,
+      });
+    const canDetectMention = mentionRegexes.length > 0;
+    const mentionGate = resolveMentionGatingWithBypass({
+      isGroup,
+      requireMention: Boolean(requireMention),
+      canDetectMention,
+      wasMentioned,
+      implicitMention: false,
+      hasAnyMention: false,
+      allowTextCommands: true,
+      hasControlCommand: hasControlCommandInMessage,
+      commandAuthorized,
+    });
+    const effectiveWasMentioned = mentionGate.effectiveWasMentioned;
+    if (isGroup && requireMention && canDetectMention && mentionGate.shouldSkip) {
+      logInboundDrop({
+        log: logVerbose,
+        channel: "signal",
+        reason: "no mention",
+        target: senderDisplay,
+      });
+      const quoteText = dataMessage.quote?.text?.trim() || "";
+      const pendingPlaceholder = (() => {
+        if (!dataMessage.attachments?.length) {
+          return "";
+        }
+        // When we're skipping a message we intentionally avoid downloading attachments.
+        // Still record a useful placeholder for pending-history context.
+        if (deps.ignoreAttachments) {
+          return "<media:attachment>";
+        }
+        const firstContentType = dataMessage.attachments?.[0]?.contentType;
+        const pendingKind = mediaKindFromMime(firstContentType ?? undefined);
+        return pendingKind ? `<media:${pendingKind}>` : "<media:attachment>";
+      })();
+      const pendingBodyText = messageText || pendingPlaceholder || quoteText;
+      const historyKey = groupId ?? "unknown";
+      recordPendingHistoryEntryIfEnabled({
+        historyMap: deps.groupHistories,
+        historyKey,
+        limit: deps.historyLimit,
+        entry: {
+          sender: envelope.sourceName ?? senderDisplay,
+          body: pendingBodyText,
+          timestamp: envelope.timestamp ?? undefined,
+          messageId:
+            typeof envelope.timestamp === "number" ? String(envelope.timestamp) : undefined,
+        },
+      });
+      return;
+    }
+
    let mediaPath: string | undefined;
    let mediaType: string | undefined;
    let placeholder = "";
@@ -576,6 +652,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
      mediaPath,
      mediaType,
      commandAuthorized,
+      wasMentioned: effectiveWasMentioned,
    });
  };
 }