fix(signal): match group allowlists against group ids

CHANGELOG.md

@@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai
 - Plugins/runtime-deps: keep bundled provider policy config loading from staging plugin runtime dependencies, so config reads no longer fail on locked-down `/var/lib/openclaw/plugin-runtime-deps` directories. Fixes #74971. Thanks @eurojojo.
 - Memory/runtime-deps: retain the native `node-llama-cpp` runtime only when local memory search is configured, so packaged installs can repair local embeddings without relying on unreachable global npm installs. Fixes #74777. Thanks @LLagoon3.
 - Gateway/startup: skip pre-bind web-fetch provider discovery for credential-free `tools.web.fetch` config, so Docker/Kubernetes gateways bind even when optional fetch limits are present. Fixes #74896. Thanks @KoykL.
+- Signal: match group allowlists against inbound Signal group ids as well as sender ids, so configured groups no longer drop every message before mention policy runs. Part of #53308. Thanks @minupla and @juan-flores077.
 - Signal: bound `signal-cli` installer release and archive downloads with explicit timeouts, declared and streamed size checks, and partial-file cleanup. Fixes #54153. Thanks @jinduwang1001-max and @juan-flores077.
 - Slack: require bot-authored room messages with `allowBots=true` to come from an explicitly channel-allowlisted bot or from a room where an explicit Slack owner is present, so broad bot relays cannot run unattended. Fixes #59284. Thanks @andrewhong-translucent.
 - Signal: derive `getAttachment` HTTP response caps from `channels.signal.mediaMaxMb` with base64 headroom, so inbound photos and videos no longer drop behind the 1 MiB RPC default. Fixes #73564. Thanks @heyhudson.

docs/channels/groups.md

@@ -257,6 +257,7 @@ Control how group/room messages are handled per channel:
   <Accordion title="Per-channel notes">
     - `groupPolicy` is separate from mention-gating (which requires @mentions).
     - WhatsApp/Telegram/Signal/iMessage/Microsoft Teams/Zalo: use `groupAllowFrom` (fallback: explicit `allowFrom`).
+    - Signal: `groupAllowFrom` can match either the inbound Signal group id or the sender phone/UUID.
     - DM pairing approvals (`*-allowFrom` store entries) apply to DM access only; group sender authorization stays explicit to group allowlists.
     - Discord: allowlist uses `channels.discord.guilds.<id>.channels`.
     - Slack: allowlist uses `channels.slack.channels`.

docs/channels/signal.md

@@ -194,7 +194,7 @@ DMs:
 Groups:
 
 - `channels.signal.groupPolicy = open | allowlist | disabled`.
-- `channels.signal.groupAllowFrom` controls who can trigger in groups when `allowlist` is set.
+- `channels.signal.groupAllowFrom` controls which groups or senders can trigger group replies when `allowlist` is set; entries can be Signal group IDs (raw, `group:<id>`, or `signal:group:<id>`), sender phone numbers, or `uuid:<id>` values.
 - `channels.signal.groups["<group-id>" | "*"]` can override group behavior with `requireMention`, `tools`, and `toolsBySender`.
 - Use `channels.signal.accounts.<id>.groups` for per-account overrides in multi-account setups.
 - Runtime note: if `channels.signal` is completely missing, runtime falls back to `groupPolicy="allowlist"` for group checks (even if `channels.defaults.groupPolicy` is set).
@@ -314,7 +314,7 @@ Provider options:
 - `channels.signal.dmPolicy`: `pairing | allowlist | open | disabled` (default: pairing).
 - `channels.signal.allowFrom`: DM allowlist (E.164 or `uuid:<id>`). `open` requires `"*"`. Signal has no usernames; use phone/UUID ids.
 - `channels.signal.groupPolicy`: `open | allowlist | disabled` (default: allowlist).
-- `channels.signal.groupAllowFrom`: group sender allowlist.
+- `channels.signal.groupAllowFrom`: group allowlist; accepts Signal group IDs (raw, `group:<id>`, or `signal:group:<id>`), sender E.164 numbers, or `uuid:<id>` values.
 - `channels.signal.groups`: per-group overrides keyed by Signal group id (or `"*"`). Supported fields: `requireMention`, `tools`, `toolsBySender`.
 - `channels.signal.accounts.<id>.groups`: per-account version of `channels.signal.groups` for multi-account setups.
 - `channels.signal.historyLimit`: max group messages to include as context (0 disables).

extensions/signal/src/monitor/access-policy.test.ts

@@ -1,5 +1,105 @@
 import { describe, expect, it, vi } from "vitest";
-import { handleSignalDirectMessageAccess } from "./access-policy.js";
+import { handleSignalDirectMessageAccess, resolveSignalAccessState } from "./access-policy.js";
+
+vi.mock("openclaw/plugin-sdk/security-runtime", async (importOriginal) => ({
+  ...(await importOriginal<typeof import("openclaw/plugin-sdk/security-runtime")>()),
+  readStoreAllowFromForDmPolicy: vi.fn(async () => []),
+}));
+
+const SIGNAL_GROUP_ID = "signal-group-id";
+const OTHER_SIGNAL_GROUP_ID = "other-signal-group-id";
+const SIGNAL_SENDER = {
+  kind: "phone" as const,
+  e164: "+15551230000",
+  raw: "+15551230000",
+};
+
+async function resolveGroupAccess(params: {
+  allowFrom?: string[];
+  groupAllowFrom?: string[];
+  groupId?: string;
+}) {
+  const access = await resolveSignalAccessState({
+    accountId: "default",
+    dmPolicy: "allowlist",
+    groupPolicy: "allowlist",
+    allowFrom: params.allowFrom ?? [],
+    groupAllowFrom: params.groupAllowFrom ?? [],
+    sender: SIGNAL_SENDER,
+    groupId: params.groupId,
+  });
+  return {
+    ...access,
+    groupDecision: access.resolveAccessDecision(true),
+  };
+}
+
+describe("resolveSignalAccessState", () => {
+  it("allows group messages when groupAllowFrom contains the inbound Signal group id", async () => {
+    const { groupDecision } = await resolveGroupAccess({
+      groupAllowFrom: [SIGNAL_GROUP_ID],
+      groupId: SIGNAL_GROUP_ID,
+    });
+
+    expect(groupDecision.decision).toBe("allow");
+  });
+
+  it("allows Signal group target forms in groupAllowFrom", async () => {
+    const groupTargetDecision = await resolveGroupAccess({
+      groupAllowFrom: [`group:${SIGNAL_GROUP_ID}`],
+      groupId: SIGNAL_GROUP_ID,
+    });
+    const signalGroupTargetDecision = await resolveGroupAccess({
+      groupAllowFrom: [`signal:group:${SIGNAL_GROUP_ID}`],
+      groupId: SIGNAL_GROUP_ID,
+    });
+
+    expect(groupTargetDecision.groupDecision.decision).toBe("allow");
+    expect(signalGroupTargetDecision.groupDecision.decision).toBe("allow");
+  });
+
+  it("blocks group messages when groupAllowFrom contains a different Signal group id", async () => {
+    const { groupDecision } = await resolveGroupAccess({
+      groupAllowFrom: [OTHER_SIGNAL_GROUP_ID],
+      groupId: SIGNAL_GROUP_ID,
+    });
+
+    expect(groupDecision.decision).toBe("block");
+  });
+
+  it("keeps sender allowlist compatibility for Signal group messages", async () => {
+    const { groupDecision } = await resolveGroupAccess({
+      groupAllowFrom: [SIGNAL_SENDER.e164],
+      groupId: SIGNAL_GROUP_ID,
+    });
+
+    expect(groupDecision.decision).toBe("allow");
+  });
+
+  it("does not match group ids against direct-message allowFrom entries", async () => {
+    const { dmAccess } = await resolveSignalAccessState({
+      accountId: "default",
+      dmPolicy: "allowlist",
+      groupPolicy: "allowlist",
+      allowFrom: [SIGNAL_GROUP_ID],
+      groupAllowFrom: [],
+      sender: SIGNAL_SENDER,
+      groupId: SIGNAL_GROUP_ID,
+    });
+
+    expect(dmAccess.decision).toBe("block");
+  });
+
+  it("does not let group ids in allowFrom satisfy an explicit groupAllowFrom mismatch", async () => {
+    const { groupDecision } = await resolveGroupAccess({
+      allowFrom: [SIGNAL_GROUP_ID],
+      groupAllowFrom: [OTHER_SIGNAL_GROUP_ID],
+      groupId: SIGNAL_GROUP_ID,
+    });
+
+    expect(groupDecision.decision).toBe("block");
+  });
+});
 
 describe("handleSignalDirectMessageAccess", () => {
   it("returns true for already-allowed direct messages", async () => {

extensions/signal/src/monitor/access-policy.ts

@@ -9,6 +9,14 @@ import { isSignalSenderAllowed, type SignalSender } from "../identity.js";
 type SignalDmPolicy = "open" | "pairing" | "allowlist" | "disabled";
 type SignalGroupPolicy = "open" | "allowlist" | "disabled";
 
+function isSignalGroupAllowed(groupId: string | undefined, allowEntries: string[]): boolean {
+  if (!groupId) {
+    return false;
+  }
+  const candidates = new Set([groupId, `group:${groupId}`, `signal:group:${groupId}`]);
+  return allowEntries.some((entry) => candidates.has(entry));
+}
+
 export async function resolveSignalAccessState(params: {
   accountId: string;
   dmPolicy: SignalDmPolicy;
@@ -16,12 +24,17 @@ export async function resolveSignalAccessState(params: {
   allowFrom: string[];
   groupAllowFrom: string[];
   sender: SignalSender;
+  groupId?: string;
 }) {
   const storeAllowFrom = await readStoreAllowFromForDmPolicy({
     provider: "signal",
     accountId: params.accountId,
     dmPolicy: params.dmPolicy,
   });
+  const isSenderAllowed = (allowEntries: string[]) =>
+    isSignalSenderAllowed(params.sender, allowEntries);
+  const isSenderOrGroupAllowed = (allowEntries: string[]) =>
+    isSenderAllowed(allowEntries) || isSignalGroupAllowed(params.groupId, allowEntries);
   const resolveAccessDecision = (isGroup: boolean) =>
     resolveDmGroupAccessWithLists({
       isGroup,
@@ -30,11 +43,12 @@ export async function resolveSignalAccessState(params: {
       allowFrom: params.allowFrom,
       groupAllowFrom: params.groupAllowFrom,
       storeAllowFrom,
-      isSenderAllowed: (allowEntries) => isSignalSenderAllowed(params.sender, allowEntries),
+      isSenderAllowed: isGroup ? isSenderOrGroupAllowed : isSenderAllowed,
     });
   const dmAccess = resolveAccessDecision(false);
   return {
     resolveAccessDecision,
+    isGroupAllowed: isSenderOrGroupAllowed,
     dmAccess,
     effectiveDmAllow: dmAccess.effectiveAllowFrom,
     effectiveGroupAllow: dmAccess.effectiveGroupAllowFrom,

extensions/signal/src/monitor/event-handler.inbound-context.test.ts

@@ -1,32 +1,38 @@
 import { expectChannelInboundContextContract as expectInboundContextContract } from "openclaw/plugin-sdk/channel-contract-testing";
 import type { MsgContext } from "openclaw/plugin-sdk/reply-runtime";
 import { beforeEach, describe, expect, it, vi } from "vitest";
+import type { SignalReactionMessage } from "./event-handler.types.js";
 vi.useRealTimers();
 const [
   { createBaseSignalEventHandlerDeps, createSignalReceiveEvent },
   { createSignalEventHandler },
 ] = await Promise.all([import("./event-handler.test-harness.js"), import("./event-handler.js")]);
 
-const { sendTypingMock, sendReadReceiptMock, dispatchInboundMessageMock, capture } = vi.hoisted(
-  () => {
-    const captureState: { ctx?: MsgContext } = {};
-    return {
-      sendTypingMock: vi.fn(),
-      sendReadReceiptMock: vi.fn(),
-      dispatchInboundMessageMock: vi.fn(
-        async (params: {
-          ctx: MsgContext;
-          replyOptions?: { onReplyStart?: () => void | Promise<void> };
-        }) => {
-          captureState.ctx = params.ctx;
-          await Promise.resolve(params.replyOptions?.onReplyStart?.());
-          return { queuedFinal: false, counts: { tool: 0, block: 0, final: 0 } };
-        },
-      ),
-      capture: captureState,
-    };
-  },
-);
+const {
+  sendTypingMock,
+  sendReadReceiptMock,
+  dispatchInboundMessageMock,
+  enqueueSystemEventMock,
+  capture,
+} = vi.hoisted(() => {
+  const captureState: { ctx?: MsgContext } = {};
+  return {
+    sendTypingMock: vi.fn(),
+    sendReadReceiptMock: vi.fn(),
+    enqueueSystemEventMock: vi.fn(),
+    dispatchInboundMessageMock: vi.fn(
+      async (params: {
+        ctx: MsgContext;
+        replyOptions?: { onReplyStart?: () => void | Promise<void> };
+      }) => {
+        captureState.ctx = params.ctx;
+        await Promise.resolve(params.replyOptions?.onReplyStart?.());
+        return { queuedFinal: false, counts: { tool: 0, block: 0, final: 0 } };
+      },
+    ),
+    capture: captureState,
+  };
+});
 
 vi.mock("../send.js", () => ({
   sendMessageSignal: vi.fn(),
@@ -57,11 +63,22 @@ vi.mock("openclaw/plugin-sdk/conversation-runtime", async () => {
   };
 });
 
+vi.mock("openclaw/plugin-sdk/system-event-runtime", async () => {
+  const actual = await vi.importActual<typeof import("openclaw/plugin-sdk/system-event-runtime")>(
+    "openclaw/plugin-sdk/system-event-runtime",
+  );
+  return {
+    ...actual,
+    enqueueSystemEvent: enqueueSystemEventMock,
+  };
+});
+
 describe("signal createSignalEventHandler inbound context", () => {
   beforeEach(() => {
     delete capture.ctx;
     sendTypingMock.mockReset().mockResolvedValue(true);
     sendReadReceiptMock.mockReset().mockResolvedValue(true);
+    enqueueSystemEventMock.mockReset();
     dispatchInboundMessageMock.mockClear();
   });
 
@@ -197,6 +214,154 @@ describe("signal createSignalEventHandler inbound context", () => {
     expect(dispatchInboundMessageMock).not.toHaveBeenCalled();
   });
 
+  it("allows Signal groups whose id is listed in groupAllowFrom", async () => {
+    const handler = createSignalEventHandler(
+      createBaseSignalEventHandlerDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 } },
+          channels: {
+            signal: {
+              groupPolicy: "allowlist",
+              groupAllowFrom: ["g1"],
+              groups: { "*": { requireMention: false } },
+            },
+          },
+        },
+        groupPolicy: "allowlist",
+        groupAllowFrom: ["g1"],
+        historyLimit: 0,
+      }),
+    );
+
+    await handler(
+      createSignalReceiveEvent({
+        dataMessage: {
+          message: "hello from allowed group",
+          groupInfo: { groupId: "g1", groupName: "Test Group" },
+          attachments: [],
+        },
+      }),
+    );
+
+    expect(capture.ctx).toBeTruthy();
+    expect(capture.ctx?.ChatType).toBe("group");
+    expect(capture.ctx?.From).toBe("group:g1");
+  });
+
+  it("blocks Signal groups whose id is not listed in groupAllowFrom", async () => {
+    const handler = createSignalEventHandler(
+      createBaseSignalEventHandlerDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 } },
+          channels: {
+            signal: {
+              groupPolicy: "allowlist",
+              groupAllowFrom: ["g2"],
+              groups: { "*": { requireMention: false } },
+            },
+          },
+        },
+        groupPolicy: "allowlist",
+        groupAllowFrom: ["g2"],
+        historyLimit: 0,
+      }),
+    );
+
+    await handler(
+      createSignalReceiveEvent({
+        dataMessage: {
+          message: "hello from blocked group",
+          groupInfo: { groupId: "g1", groupName: "Test Group" },
+          attachments: [],
+        },
+      }),
+    );
+
+    expect(capture.ctx).toBeUndefined();
+    expect(dispatchInboundMessageMock).not.toHaveBeenCalled();
+  });
+
+  it("authorizes group control commands when groupAllowFrom matches the Signal group id", async () => {
+    const handler = createSignalEventHandler(
+      createBaseSignalEventHandlerDeps({
+        cfg: {
+          messages: {
+            inbound: { debounceMs: 0 },
+            groupChat: { mentionPatterns: ["@bot"] },
+          },
+          channels: {
+            signal: {
+              groupPolicy: "allowlist",
+              groupAllowFrom: ["g1"],
+              groups: { "*": { requireMention: true } },
+            },
+          },
+        },
+        groupPolicy: "allowlist",
+        groupAllowFrom: ["g1"],
+        historyLimit: 0,
+      }),
+    );
+
+    await handler(
+      createSignalReceiveEvent({
+        dataMessage: {
+          message: "/status",
+          groupInfo: { groupId: "g1", groupName: "Test Group" },
+          attachments: [],
+        },
+      }),
+    );
+
+    expect(capture.ctx).toBeTruthy();
+    expect(capture.ctx?.CommandAuthorized).toBe(true);
+  });
+
+  it("allows reaction-only group events when groupAllowFrom matches the reaction group id", async () => {
+    const handler = createSignalEventHandler(
+      createBaseSignalEventHandlerDeps({
+        cfg: {
+          messages: { inbound: { debounceMs: 0 } },
+          channels: {
+            signal: {
+              groupPolicy: "allowlist",
+              groupAllowFrom: ["g1"],
+            },
+          },
+        },
+        groupPolicy: "allowlist",
+        groupAllowFrom: ["g1"],
+        reactionMode: "all",
+        isSignalReactionMessage: (reaction): reaction is SignalReactionMessage => Boolean(reaction),
+        shouldEmitSignalReactionNotification: () => true,
+        resolveSignalReactionTargets: () => [
+          { kind: "phone", id: "+15550001111", display: "+15550001111" },
+        ],
+        buildSignalReactionSystemEventText: () => "reaction added",
+        historyLimit: 0,
+      }),
+    );
+
+    await handler(
+      createSignalReceiveEvent({
+        reactionMessage: {
+          emoji: "+1",
+          targetSentTimestamp: 1700000000000,
+          groupInfo: { groupId: "g1", groupName: "Test Group" },
+        },
+      }),
+    );
+
+    expect(dispatchInboundMessageMock).not.toHaveBeenCalled();
+    expect(enqueueSystemEventMock).toHaveBeenCalledWith(
+      "reaction added",
+      expect.objectContaining({
+        sessionKey: "agent:main:signal:group:g1",
+        trusted: false,
+      }),
+    );
+  });
+
   it("drops quote-only group context from non-allowlisted quoted senders in allowlist mode", async () => {
     const handler = createSignalEventHandler(
       createBaseSignalEventHandlerDeps({

extensions/signal/src/monitor/event-handler.ts

@@ -552,19 +552,25 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
     const rawMessage = dataMessage?.message ?? "";
     const normalizedMessage = renderSignalMentions(rawMessage, dataMessage?.mentions);
     const messageText = normalizedMessage.trim();
-    const groupId = dataMessage?.groupInfo?.groupId ?? undefined;
+    const groupId = dataMessage?.groupInfo?.groupId ?? reaction?.groupInfo?.groupId ?? undefined;
     const isGroup = Boolean(groupId);
 
     const senderDisplay = formatSignalSenderDisplay(sender);
-    const { resolveAccessDecision, dmAccess, effectiveDmAllow, effectiveGroupAllow } =
-      await resolveSignalAccessState({
-        accountId: deps.accountId,
-        dmPolicy: deps.dmPolicy,
-        groupPolicy: deps.groupPolicy,
-        allowFrom: deps.allowFrom,
-        groupAllowFrom: deps.groupAllowFrom,
-        sender,
-      });
+    const {
+      resolveAccessDecision,
+      isGroupAllowed,
+      dmAccess,
+      effectiveDmAllow,
+      effectiveGroupAllow,
+    } = await resolveSignalAccessState({
+      accountId: deps.accountId,
+      dmPolicy: deps.dmPolicy,
+      groupPolicy: deps.groupPolicy,
+      allowFrom: deps.allowFrom,
+      groupAllowFrom: deps.groupAllowFrom,
+      sender,
+      groupId,
+    });
     const quoteText = normalizeOptionalString(dataMessage?.quote?.text) ?? "";
     const { contextVisibilityMode, quoteSenderAllowed, visibleQuoteText, visibleQuoteSender } =
       resolveSignalQuoteContext({
@@ -650,7 +656,7 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
     const useAccessGroups = deps.cfg.commands?.useAccessGroups !== false;
     const commandDmAllow = isGroup ? deps.allowFrom : effectiveDmAllow;
     const ownerAllowedForCommands = isSignalSenderAllowed(sender, commandDmAllow);
-    const groupAllowedForCommands = isSignalSenderAllowed(sender, effectiveGroupAllow);
+    const groupAllowedForCommands = isGroupAllowed(effectiveGroupAllow);
     const hasControlCommandInMessage = hasControlCommand(messageText, deps.cfg);
     const commandGate = resolveControlCommandGate({
       useAccessGroups,