fix(signal): add group-level allowlist support via groups config
Signal's groupPolicy: allowlist was checking senders against groupAllowFrom, but users expected to put group IDs there. This adds proper group-level allowlisting via channels.signal.groups.<groupId>, matching the pattern used by Telegram and iMessage. When a group is explicitly listed in the groups config, it bypasses the sender-level groupAllowFrom check, allowing all members of that group to interact with the bot. - Add SignalGroupConfig type and groups property to SignalAccountConfig - Add resolveChannelGroupPolicy check before sender-level gating - Add tests for group-level allowlist functionality - Update docs to clarify groupAllowFrom vs groups config
This commit is contained in:
parent
4583f88626
commit
c9359b8486
@ -102,7 +102,24 @@ DMs:
|
|||||||
|
|
||||||
Groups:
|
Groups:
|
||||||
- `channels.signal.groupPolicy = open | allowlist | disabled`.
|
- `channels.signal.groupPolicy = open | allowlist | disabled`.
|
||||||
- `channels.signal.groupAllowFrom` controls who can trigger in groups when `allowlist` is set.
|
- `channels.signal.groupAllowFrom` controls which **senders** (phone numbers or UUIDs) can trigger in groups when `allowlist` is set.
|
||||||
|
- `channels.signal.groups.<groupId>` explicitly allows specific **groups** by ID. When a group is listed here, it bypasses the sender-level `groupAllowFrom` check.
|
||||||
|
|
||||||
|
Example allowing a specific group:
|
||||||
|
```json5
|
||||||
|
{
|
||||||
|
channels: {
|
||||||
|
signal: {
|
||||||
|
groupPolicy: "allowlist",
|
||||||
|
groups: {
|
||||||
|
"your-signal-group-id-here": {} // Empty object = allowed
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
```
|
||||||
|
|
||||||
|
To find your Signal group ID, check the gateway logs when a message arrives from that group, or use `moltbot channels status --deep`.
|
||||||
|
|
||||||
## How it works (behavior)
|
## How it works (behavior)
|
||||||
- `signal-cli` runs as a daemon; the gateway reads events via SSE.
|
- `signal-cli` runs as a daemon; the gateway reads events via SSE.
|
||||||
@ -166,7 +183,8 @@ Provider options:
|
|||||||
- `channels.signal.dmPolicy`: `pairing | allowlist | open | disabled` (default: pairing).
|
- `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.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.groupPolicy`: `open | allowlist | disabled` (default: allowlist).
|
||||||
- `channels.signal.groupAllowFrom`: group sender allowlist.
|
- `channels.signal.groupAllowFrom`: group sender allowlist (E.164 or `uuid:<id>`).
|
||||||
|
- `channels.signal.groups.<groupId>`: per-group config; presence in this map allows the group (bypasses `groupAllowFrom` sender check).
|
||||||
- `channels.signal.historyLimit`: max group messages to include as context (0 disables).
|
- `channels.signal.historyLimit`: max group messages to include as context (0 disables).
|
||||||
- `channels.signal.dmHistoryLimit`: DM history limit in user turns. Per-user overrides: `channels.signal.dms["<phone_or_uuid>"].historyLimit`.
|
- `channels.signal.dmHistoryLimit`: DM history limit in user turns. Per-user overrides: `channels.signal.dms["<phone_or_uuid>"].historyLimit`.
|
||||||
- `channels.signal.textChunkLimit`: outbound chunk size (chars).
|
- `channels.signal.textChunkLimit`: outbound chunk size (chars).
|
||||||
|
|||||||
@ -6,10 +6,26 @@ import type {
|
|||||||
} from "./types.base.js";
|
} from "./types.base.js";
|
||||||
import type { ChannelHeartbeatVisibilityConfig } from "./types.channels.js";
|
import type { ChannelHeartbeatVisibilityConfig } from "./types.channels.js";
|
||||||
import type { DmConfig } from "./types.messages.js";
|
import type { DmConfig } from "./types.messages.js";
|
||||||
|
import type { GroupToolPolicyBySenderConfig, GroupToolPolicyConfig } from "./types.tools.js";
|
||||||
|
|
||||||
export type SignalReactionNotificationMode = "off" | "own" | "all" | "allowlist";
|
export type SignalReactionNotificationMode = "off" | "own" | "all" | "allowlist";
|
||||||
export type SignalReactionLevel = "off" | "ack" | "minimal" | "extensive";
|
export type SignalReactionLevel = "off" | "ack" | "minimal" | "extensive";
|
||||||
|
|
||||||
|
export type SignalGroupConfig = {
|
||||||
|
/** If true, require @mention to respond in this group. */
|
||||||
|
requireMention?: boolean;
|
||||||
|
/** Optional tool policy overrides for this group. */
|
||||||
|
tools?: GroupToolPolicyConfig;
|
||||||
|
/** Per-sender tool policy overrides. */
|
||||||
|
toolsBySender?: GroupToolPolicyBySenderConfig;
|
||||||
|
/** If false, disable the bot for this group. */
|
||||||
|
enabled?: boolean;
|
||||||
|
/** Optional allowlist for group senders (E.164 or uuid:<id>). */
|
||||||
|
allowFrom?: Array<string | number>;
|
||||||
|
/** Optional system prompt snippet for this group. */
|
||||||
|
systemPrompt?: string;
|
||||||
|
};
|
||||||
|
|
||||||
export type SignalAccountConfig = {
|
export type SignalAccountConfig = {
|
||||||
/** Optional display name for this account (used in CLI/UI lists). */
|
/** Optional display name for this account (used in CLI/UI lists). */
|
||||||
name?: string;
|
name?: string;
|
||||||
@ -51,6 +67,8 @@ export type SignalAccountConfig = {
|
|||||||
* - "allowlist": only allow group messages from senders in groupAllowFrom/allowFrom
|
* - "allowlist": only allow group messages from senders in groupAllowFrom/allowFrom
|
||||||
*/
|
*/
|
||||||
groupPolicy?: GroupPolicy;
|
groupPolicy?: GroupPolicy;
|
||||||
|
/** Per-group configuration keyed by Signal group ID. */
|
||||||
|
groups?: Record<string, SignalGroupConfig>;
|
||||||
/** Max group messages to keep as history context (0 disables). */
|
/** Max group messages to keep as history context (0 disables). */
|
||||||
historyLimit?: number;
|
historyLimit?: number;
|
||||||
/** Max DM turns to keep as history context. */
|
/** Max DM turns to keep as history context. */
|
||||||
|
|||||||
156
src/signal/monitor/event-handler.group-allowlist.test.ts
Normal file
156
src/signal/monitor/event-handler.group-allowlist.test.ts
Normal file
@ -0,0 +1,156 @@
|
|||||||
|
import { describe, expect, it, vi } from "vitest";
|
||||||
|
|
||||||
|
import type { MsgContext } from "../../auto-reply/templating.js";
|
||||||
|
|
||||||
|
let capturedCtx: MsgContext | undefined;
|
||||||
|
let dispatchCalled = false;
|
||||||
|
|
||||||
|
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;
|
||||||
|
dispatchCalled = true;
|
||||||
|
return { queuedFinal: false, counts: { tool: 0, block: 0, final: 0 } };
|
||||||
|
});
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
dispatchInboundMessage,
|
||||||
|
dispatchInboundMessageWithDispatcher: dispatchInboundMessage,
|
||||||
|
dispatchInboundMessageWithBufferedDispatcher: dispatchInboundMessage,
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
import { createSignalEventHandler } from "./event-handler.js";
|
||||||
|
|
||||||
|
describe("signal group-level allowlist (groups config)", () => {
|
||||||
|
const makeHandler = (cfg: Record<string, unknown>) =>
|
||||||
|
createSignalEventHandler({
|
||||||
|
runtime: { log: () => {}, error: () => {} } as any,
|
||||||
|
cfg: { messages: { inbound: { debounceMs: 0 } }, ...cfg } as any,
|
||||||
|
baseUrl: "http://localhost",
|
||||||
|
accountId: "default",
|
||||||
|
historyLimit: 0,
|
||||||
|
groupHistories: new Map(),
|
||||||
|
textLimit: 4000,
|
||||||
|
dmPolicy: "open",
|
||||||
|
allowFrom: ["+15559999999"],
|
||||||
|
groupAllowFrom: [],
|
||||||
|
groupPolicy: "allowlist",
|
||||||
|
reactionMode: "off",
|
||||||
|
reactionAllowlist: [],
|
||||||
|
mediaMaxBytes: 1024,
|
||||||
|
ignoreAttachments: true,
|
||||||
|
sendReadReceipts: false,
|
||||||
|
readReceiptsViaDaemon: false,
|
||||||
|
fetchAttachment: async () => null,
|
||||||
|
deliverReplies: async () => {},
|
||||||
|
resolveSignalReactionTargets: () => [],
|
||||||
|
isSignalReactionMessage: () => false as any,
|
||||||
|
shouldEmitSignalReactionNotification: () => false,
|
||||||
|
buildSignalReactionSystemEventText: () => "reaction",
|
||||||
|
});
|
||||||
|
|
||||||
|
const sendGroupMessage = async (
|
||||||
|
handler: ReturnType<typeof makeHandler>,
|
||||||
|
groupId: string,
|
||||||
|
sender = "+15550001111",
|
||||||
|
) => {
|
||||||
|
capturedCtx = undefined;
|
||||||
|
dispatchCalled = false;
|
||||||
|
await handler({
|
||||||
|
event: "receive",
|
||||||
|
data: JSON.stringify({
|
||||||
|
envelope: {
|
||||||
|
sourceNumber: sender,
|
||||||
|
sourceName: "Alice",
|
||||||
|
timestamp: 1700000000000,
|
||||||
|
dataMessage: {
|
||||||
|
message: "hi",
|
||||||
|
attachments: [],
|
||||||
|
groupInfo: { groupId, groupName: "Test Group" },
|
||||||
|
},
|
||||||
|
},
|
||||||
|
}),
|
||||||
|
});
|
||||||
|
};
|
||||||
|
|
||||||
|
it("allows group messages when group is in channels.signal.groups config", async () => {
|
||||||
|
const handler = makeHandler({
|
||||||
|
channels: {
|
||||||
|
signal: {
|
||||||
|
groups: {
|
||||||
|
"allowed-group-id": {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await sendGroupMessage(handler, "allowed-group-id");
|
||||||
|
|
||||||
|
expect(dispatchCalled).toBe(true);
|
||||||
|
expect(capturedCtx).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("blocks group messages when group is not in channels.signal.groups config", async () => {
|
||||||
|
const handler = makeHandler({
|
||||||
|
channels: {
|
||||||
|
signal: {
|
||||||
|
groups: {
|
||||||
|
"allowed-group-id": {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await sendGroupMessage(handler, "not-allowed-group-id");
|
||||||
|
|
||||||
|
expect(dispatchCalled).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("allows all groups when groups config has wildcard entry", async () => {
|
||||||
|
const handler = makeHandler({
|
||||||
|
channels: {
|
||||||
|
signal: {
|
||||||
|
groups: {
|
||||||
|
"*": {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
await sendGroupMessage(handler, "any-group-id");
|
||||||
|
|
||||||
|
expect(dispatchCalled).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("bypasses sender-level groupAllowFrom check when group is explicitly allowed", async () => {
|
||||||
|
// groupAllowFrom is empty, which would normally block all senders
|
||||||
|
// But the group is explicitly allowed via groups config, so it should pass
|
||||||
|
const handler = makeHandler({
|
||||||
|
channels: {
|
||||||
|
signal: {
|
||||||
|
groups: {
|
||||||
|
"family-chat": {},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
},
|
||||||
|
});
|
||||||
|
|
||||||
|
// Sender +15550001111 is NOT in groupAllowFrom (which is empty)
|
||||||
|
// But the group "family-chat" is in groups config, so message should pass
|
||||||
|
await sendGroupMessage(handler, "family-chat", "+15550001111");
|
||||||
|
|
||||||
|
expect(dispatchCalled).toBe(true);
|
||||||
|
expect(capturedCtx).toBeTruthy();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("falls back to sender-level check when groups config is not set", async () => {
|
||||||
|
// No groups config, groupPolicy is allowlist, groupAllowFrom is empty
|
||||||
|
// Should block because sender is not in allowlist
|
||||||
|
const handler = makeHandler({});
|
||||||
|
|
||||||
|
await sendGroupMessage(handler, "any-group");
|
||||||
|
|
||||||
|
expect(dispatchCalled).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
@ -20,6 +20,7 @@ import { logInboundDrop, logTypingFailure } from "../../channels/logging.js";
|
|||||||
import { createReplyPrefixContext } from "../../channels/reply-prefix.js";
|
import { createReplyPrefixContext } from "../../channels/reply-prefix.js";
|
||||||
import { recordInboundSession } from "../../channels/session.js";
|
import { recordInboundSession } from "../../channels/session.js";
|
||||||
import { createTypingCallbacks } from "../../channels/typing.js";
|
import { createTypingCallbacks } from "../../channels/typing.js";
|
||||||
|
import { resolveChannelGroupPolicy } from "../../config/group-policy.js";
|
||||||
import { readSessionUpdatedAt, resolveStorePath } from "../../config/sessions.js";
|
import { readSessionUpdatedAt, resolveStorePath } from "../../config/sessions.js";
|
||||||
import { danger, logVerbose, shouldLogVerbose } from "../../globals.js";
|
import { danger, logVerbose, shouldLogVerbose } from "../../globals.js";
|
||||||
import { enqueueSystemEvent } from "../../infra/system-events.js";
|
import { enqueueSystemEvent } from "../../infra/system-events.js";
|
||||||
@ -431,7 +432,30 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
|||||||
logVerbose("Blocked signal group message (groupPolicy: disabled)");
|
logVerbose("Blocked signal group message (groupPolicy: disabled)");
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
if (isGroup && deps.groupPolicy === "allowlist") {
|
|
||||||
|
// Group-level allowlist: check if this specific group is explicitly allowed via groups config.
|
||||||
|
// This is checked BEFORE sender-level gating so explicitly allowed groups can bypass sender checks.
|
||||||
|
// Configure via channels.signal.groups.<groupId> to explicitly allow specific groups.
|
||||||
|
const groupListPolicy =
|
||||||
|
isGroup && groupId
|
||||||
|
? resolveChannelGroupPolicy({
|
||||||
|
cfg: deps.cfg,
|
||||||
|
channel: "signal",
|
||||||
|
accountId: deps.accountId,
|
||||||
|
groupId,
|
||||||
|
})
|
||||||
|
: { allowlistEnabled: false, allowed: true };
|
||||||
|
|
||||||
|
// If groups allowlist is configured and this group is not in it, block.
|
||||||
|
if (isGroup && groupListPolicy.allowlistEnabled && !groupListPolicy.allowed) {
|
||||||
|
logVerbose(`Blocked signal group message (group ${groupId} not in groups allowlist)`);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// If group is explicitly allowed via groups config, bypass sender-level gating.
|
||||||
|
// Otherwise, apply sender-level gating when groupPolicy is "allowlist".
|
||||||
|
const groupExplicitlyAllowed = groupListPolicy.allowlistEnabled && groupListPolicy.allowed;
|
||||||
|
if (isGroup && deps.groupPolicy === "allowlist" && !groupExplicitlyAllowed) {
|
||||||
if (effectiveGroupAllow.length === 0) {
|
if (effectiveGroupAllow.length === 0) {
|
||||||
logVerbose("Blocked signal group message (groupPolicy: allowlist, no groupAllowFrom)");
|
logVerbose("Blocked signal group message (groupPolicy: allowlist, no groupAllowFrom)");
|
||||||
return;
|
return;
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user