From 1d46ca3a95c7ff2669cc9c2a231fc460a2a3cbbb Mon Sep 17 00:00:00 2001 From: zerone0x Date: Tue, 10 Feb 2026 13:19:07 +0800 Subject: [PATCH] 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 * fix(signal): keep pending history context for mention-gated skips (#13124) (thanks @zerone0x) --------- Co-authored-by: Yansu Co-authored-by: Claude Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com> --- .../event-handler.mention-gating.test.ts | 206 ++++++++++++++++++ src/signal/monitor/event-handler.ts | 77 +++++++ 2 files changed, 283 insertions(+) create mode 100644 src/signal/monitor/event-handler.mention-gating.test.ts diff --git a/src/signal/monitor/event-handler.mention-gating.test.ts b/src/signal/monitor/event-handler.mention-gating.test.ts new file mode 100644 index 0000000000..9bdf0c59be --- /dev/null +++ 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(); + 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 = {}) { + 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(""); + }); + + 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(); + }); +}); diff --git a/src/signal/monitor/event-handler.ts b/src/signal/monitor/event-handler.ts index d971392223..9b6997aa5b 100644 --- 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 ""; + } + const firstContentType = dataMessage.attachments?.[0]?.contentType; + const pendingKind = mediaKindFromMime(firstContentType ?? undefined); + return pendingKind ? `` : ""; + })(); + 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, }); }; }