diff --git a/CHANGELOG.md b/CHANGELOG.md index b3543a54045..d626a6c55ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -62,6 +62,8 @@ Docs: https://docs.openclaw.ai - NVIDIA/NIM: persist the `NVIDIA_API_KEY` provider marker and mark bundled NVIDIA Chat Completions models as string-content compatible, so NIM models load from `models.json` and OpenAI-compatible subagent calls send plain text content. Fixes #73013 and #50107; refs #73014. Thanks @bautrey, @iot2edge, @ifearghal, and @futhgar. - Channels/Discord: let text-only configs drop the `GuildVoiceStates` gateway intent and expose a bounded `/gateway/bot` metadata timeout with rate-limited fallback logs, reducing idle CPU and warning floods. Fixes #73709 and #73585. Thanks @sanchezm86 and @trac3r00. - Agents/sessions: mark same-turn `sessions_send` and A2A reply prompts with an inter-session `isUser=false` envelope before they reach the model, so foreign session output no longer lands as bare active user text. Fixes #73702; refs #73698, #73609, #73595, and #73622. Thanks @alvelda. +- Channels/Telegram: fail closed when account-level public DM settings conflict with a restrictive top-level `allowFrom`, and require an effective wildcard before `dmPolicy="open"` behaves as public access. Fixes #73756; refs #73698. Thanks @Hilo-Hilo and @xace1825. +- Channels/security: move open-DM allowlist semantics into the shared policy helpers and align Discord, Slack, Mattermost, Matrix, Feishu, LINE, IRC, Google Chat, Zalo, Zalo User, QQ Bot, and Synology Chat so `dmPolicy="open"` is public only with an effective wildcard and otherwise still respects sender allowlists. Refs #73756 and #73698. Thanks @Hilo-Hilo and @xace1825. - ACP/tasks: sweep orphaned parent-owned ACP sessions whose task records are gone, preserving bound persistent sessions but clearing unbound stale ACPX metadata so old child sessions cannot silently respawn into chat. Fixes #73609. Thanks @joerod26. - Outbound/security: strip known internal runtime scaffolding such as `` and `` at the final channel delivery boundary and keep Discord output on targeted tag stripping, so degraded harness replies cannot leak those tags to users. Fixes #73595. Thanks @gabrielexito-stack and @martingarramon. - Security/Telegram: load Telegram security adapters in read-only audit/doctor, audit malformed Telegram DM `allowFrom` entries even when groups are disabled, and keep allowlist DM audits from counting stale pairing-store senders, so public/shared-DM risk checks stay accurate. Refs #73698. Thanks @xace1825. diff --git a/docs/.generated/plugin-sdk-api-baseline.sha256 b/docs/.generated/plugin-sdk-api-baseline.sha256 index dd0fd585289..03be48b490b 100644 --- a/docs/.generated/plugin-sdk-api-baseline.sha256 +++ b/docs/.generated/plugin-sdk-api-baseline.sha256 @@ -1,2 +1,2 @@ -eedcf9070e222077f618d68510c909b571dc51fbb030284ff3b30728719f7ae0 plugin-sdk-api-baseline.json -02043e1f48a15625580ed1e1ec569ccd1c7c9ad393be2aa54a1fa36afeeca7b5 plugin-sdk-api-baseline.jsonl +21c1ddb7b6ab3da24d51971aca47b76044acf62229351dafc10ec1c0fc9ae1ff plugin-sdk-api-baseline.json +b4e011edd075864353ad238b8eeef0f6837a65f1500f21836aad7547c0c4507c plugin-sdk-api-baseline.jsonl diff --git a/docs/channels/feishu.md b/docs/channels/feishu.md index d55f81b5320..a4e09ca8936 100644 --- a/docs/channels/feishu.md +++ b/docs/channels/feishu.md @@ -45,7 +45,7 @@ Configure `dmPolicy` to control who can DM the bot: - `"pairing"` — unknown users receive a pairing code; approve via CLI - `"allowlist"` — only users listed in `allowFrom` can chat (default: bot owner only) -- `"open"` — allow all users +- `"open"` — allow public DMs only when `allowFrom` includes `"*"`; with restrictive entries, only matching users can chat - `"disabled"` — disable all DMs **Approve a pairing request:** diff --git a/docs/channels/pairing.md b/docs/channels/pairing.md index f5b9bf32a96..46ad9057bb3 100644 --- a/docs/channels/pairing.md +++ b/docs/channels/pairing.md @@ -21,6 +21,11 @@ When a channel is configured with DM policy `pairing`, unknown senders get a sho Default DM policies are documented in: [Security](/gateway/security) +`dmPolicy: "open"` is public only when the effective DM allowlist includes `"*"`. +Setup and validation require that wildcard for public-open configs. If existing +state contains `open` with concrete `allowFrom` entries, runtime still admits +only those senders, and pairing-store approvals do not widen `open` access. + Pairing codes: - 8 characters, uppercase, no ambiguous chars (`0O1I`). diff --git a/docs/channels/synology-chat.md b/docs/channels/synology-chat.md index 8a080351f91..963fd8b90f2 100644 --- a/docs/channels/synology-chat.md +++ b/docs/channels/synology-chat.md @@ -93,8 +93,8 @@ Config values override env vars. - `dmPolicy: "allowlist"` is the recommended default. - `allowedUserIds` accepts a list (or comma-separated string) of Synology user IDs. -- In `allowlist` mode, an empty `allowedUserIds` list is treated as misconfiguration and the webhook route will not start (use `dmPolicy: "open"` for allow-all). -- `dmPolicy: "open"` allows any sender. +- In `allowlist` mode, an empty `allowedUserIds` list is treated as misconfiguration and the webhook route will not start (use `dmPolicy: "open"` with `allowedUserIds: ["*"]` for allow-all). +- `dmPolicy: "open"` allows public DMs only when `allowedUserIds` includes `"*"`; with restrictive entries, only matching users can chat. - `dmPolicy: "disabled"` blocks DMs. - Reply recipient binding stays on stable numeric `user_id` by default. `channels.synology-chat.dangerouslyAllowNameMatching: true` is break-glass compatibility mode that re-enables mutable username/nickname lookup for reply delivery. - Pairing approvals work with: @@ -172,7 +172,7 @@ but duplicate exact paths are still rejected fail-closed. Prefer explicit per-ac - `Rate limit exceeded`: - too many invalid token attempts from the same source can temporarily lock that source out - authenticated senders also have a separate per-user message rate limit -- `Allowlist is empty. Configure allowedUserIds or use dmPolicy=open.`: +- `Allowlist is empty. Configure allowedUserIds or use dmPolicy=open with allowedUserIds=["*"].`: - `dmPolicy="allowlist"` is enabled but no users are configured - `User not authorized`: - the sender's numeric `user_id` is not in `allowedUserIds` diff --git a/docs/channels/telegram.md b/docs/channels/telegram.md index 7996999a780..999ca4e22f9 100644 --- a/docs/channels/telegram.md +++ b/docs/channels/telegram.md @@ -114,6 +114,7 @@ Token resolution order is account-aware. In practice, config values win over env `dmPolicy: "open"` with `allowFrom: ["*"]` lets any Telegram account that finds or guesses the bot username command the bot. Use it only for intentionally public bots with tightly restricted tools; one-owner bots should use `allowlist` with numeric user IDs. `channels.telegram.allowFrom` accepts numeric Telegram user IDs. `telegram:` / `tg:` prefixes are accepted and normalized. + In multi-account configs, a restrictive top-level `channels.telegram.allowFrom` is treated as a safety boundary: account-level `allowFrom: ["*"]` entries do not make that account public unless the effective account allowlist still contains an explicit wildcard after merging. `dmPolicy: "allowlist"` with empty `allowFrom` blocks all DMs and is rejected by config validation. Setup asks for numeric user IDs only. If you upgraded and your config contains `@username` allowlist entries, run `openclaw doctor --fix` to resolve them (best-effort; requires a Telegram bot token). diff --git a/extensions/discord/src/monitor/agent-components-helpers.ts b/extensions/discord/src/monitor/agent-components-helpers.ts index d4881471736..08e370ad6c7 100644 --- a/extensions/discord/src/monitor/agent-components-helpers.ts +++ b/extensions/discord/src/monitor/agent-components-helpers.ts @@ -491,10 +491,6 @@ async function ensureDmComponentAuthorized(params: { } catch {} return false; } - if (dmPolicy === "open") { - return true; - } - if (dmPolicy === "allowlist") { const allowMatch = resolveAllowMatch(ctx.allowFrom ?? []); if (allowMatch.allowed) { @@ -510,11 +506,14 @@ async function ensureDmComponentAuthorized(params: { return false; } - const storeAllowFrom = await readStoreAllowFromForDmPolicy({ - provider: "discord", - accountId: ctx.accountId, - dmPolicy, - }); + const storeAllowFrom = + dmPolicy === "open" + ? [] + : await readStoreAllowFromForDmPolicy({ + provider: "discord", + accountId: ctx.accountId, + dmPolicy, + }); const allowMatch = resolveAllowMatch([...(ctx.allowFrom ?? []), ...storeAllowFrom]); if (allowMatch.allowed) { return true; diff --git a/extensions/discord/src/monitor/dm-command-auth.test.ts b/extensions/discord/src/monitor/dm-command-auth.test.ts index 769d1d61666..a588d9dc9f2 100644 --- a/extensions/discord/src/monitor/dm-command-auth.test.ts +++ b/extensions/discord/src/monitor/dm-command-auth.test.ts @@ -20,11 +20,11 @@ describe("resolveDiscordDmCommandAccess", () => { }); } - it("allows open DMs and keeps command auth enabled without allowlist entries", async () => { + it("blocks open DMs without allowlist wildcard entries", async () => { const result = await resolveOpenDmAccess([]); - expect(result.decision).toBe("allow"); - expect(result.commandAuthorized).toBe(true); + expect(result.decision).toBe("block"); + expect(result.commandAuthorized).toBe(false); }); it("marks command auth true when sender is allowlisted", async () => { @@ -34,7 +34,7 @@ describe("resolveDiscordDmCommandAccess", () => { expect(result.commandAuthorized).toBe(true); }); - it("keeps command auth enabled for open DMs when configured allowlist does not match", async () => { + it("blocks open DMs when configured allowlist does not match", async () => { const result = await resolveDiscordDmCommandAccess({ accountId: "default", dmPolicy: "open", @@ -45,9 +45,9 @@ describe("resolveDiscordDmCommandAccess", () => { readStoreAllowFrom: async () => [], }); - expect(result.decision).toBe("allow"); + expect(result.decision).toBe("block"); expect(result.allowMatch.allowed).toBe(false); - expect(result.commandAuthorized).toBe(true); + expect(result.commandAuthorized).toBe(false); }); it("returns pairing decision and unauthorized command auth for unknown senders", async () => { @@ -80,7 +80,7 @@ describe("resolveDiscordDmCommandAccess", () => { expect(result.commandAuthorized).toBe(true); }); - it("keeps open DM command auth true when access groups are disabled", async () => { + it("keeps open DM blocked without wildcard even when access groups are disabled", async () => { const result = await resolveDiscordDmCommandAccess({ accountId: "default", dmPolicy: "open", @@ -91,7 +91,7 @@ describe("resolveDiscordDmCommandAccess", () => { readStoreAllowFrom: async () => [], }); - expect(result.decision).toBe("allow"); - expect(result.commandAuthorized).toBe(true); + expect(result.decision).toBe("block"); + expect(result.commandAuthorized).toBe(false); }); }); diff --git a/extensions/discord/src/monitor/dm-command-auth.ts b/extensions/discord/src/monitor/dm-command-auth.ts index b47b68ab030..2b39e2bbdca 100644 --- a/extensions/discord/src/monitor/dm-command-auth.ts +++ b/extensions/discord/src/monitor/dm-command-auth.ts @@ -33,13 +33,9 @@ function resolveSenderAllowMatch(params: { } function resolveDmPolicyCommandAuthorization(params: { - dmPolicy: DiscordDmPolicy; decision: DmGroupAccessDecision; commandAuthorized: boolean; }) { - if (params.dmPolicy === "open" && params.decision === "allow") { - return true; - } return params.commandAuthorized; } @@ -53,11 +49,14 @@ export async function resolveDiscordDmCommandAccess(params: { readStoreAllowFrom?: () => Promise; }): Promise { const storeAllowFrom = params.readStoreAllowFrom - ? await params.readStoreAllowFrom().catch(() => []) + ? params.dmPolicy === "open" + ? [] + : await params.readStoreAllowFrom().catch(() => []) : await readStoreAllowFromForDmPolicy({ provider: "discord", accountId: params.accountId, dmPolicy: params.dmPolicy, + shouldRead: params.dmPolicy !== "open", }); const access = resolveDmGroupAccessWithLists({ @@ -94,11 +93,13 @@ export async function resolveDiscordDmCommandAccess(params: { return { decision: access.decision, reason: access.reason, - commandAuthorized: resolveDmPolicyCommandAuthorization({ - dmPolicy: params.dmPolicy, - decision: access.decision, - commandAuthorized, - }), + commandAuthorized: + access.decision === "allow" + ? resolveDmPolicyCommandAuthorization({ + decision: access.decision, + commandAuthorized, + }) + : false, allowMatch, }; } diff --git a/extensions/discord/src/monitor/monitor.agent-components.test.ts b/extensions/discord/src/monitor/monitor.agent-components.test.ts index c3a5058e516..df6adb7c96f 100644 --- a/extensions/discord/src/monitor/monitor.agent-components.test.ts +++ b/extensions/discord/src/monitor/monitor.agent-components.test.ts @@ -101,11 +101,13 @@ describe("agent components", () => { async function expectSuccessfulDmButtonInteraction(params: { dmPolicy: "pairing" | "open"; expectPairingStoreRead: boolean; + allowFrom?: string[]; }) { const button = createAgentComponentButton({ cfg: createCfg(), accountId: "default", dmPolicy: params.dmPolicy, + allowFrom: params.allowFrom, }); const { interaction, defer, reply } = createDmButtonInteraction(); @@ -259,6 +261,7 @@ describe("agent components", () => { await expectSuccessfulDmButtonInteraction({ dmPolicy: "open", expectPairingStoreRead: false, + allowFrom: ["*"], }); }); diff --git a/extensions/feishu/src/bot.test.ts b/extensions/feishu/src/bot.test.ts index 4cf6acdd2db..ee6ddf7f4d6 100644 --- a/extensions/feishu/src/bot.test.ts +++ b/extensions/feishu/src/bot.test.ts @@ -338,8 +338,22 @@ vi.mock("openclaw/plugin-sdk/conversation-runtime", async () => { async function dispatchMessage(params: { cfg: ClawdbotConfig; event: FeishuMessageEvent }) { const runtime = createRuntimeEnv(); + const feishuConfig = params.cfg.channels?.feishu; + const cfg = + feishuConfig?.dmPolicy === "open" && feishuConfig.allowFrom === undefined + ? ({ + ...params.cfg, + channels: { + ...params.cfg.channels, + feishu: { + ...feishuConfig, + allowFrom: ["*"], + }, + }, + } as ClawdbotConfig) + : params.cfg; await handleFeishuMessage({ - cfg: params.cfg, + cfg, event: params.event, runtime, }); @@ -637,7 +651,7 @@ describe("handleFeishuMessage command authorization", () => { expect(mockEnqueueSystemEvent).not.toHaveBeenCalled(); }); - it("uses authorizer resolution instead of hardcoded CommandAuthorized=true", async () => { + it("blocks open DMs when a restrictive allowlist does not match", async () => { const cfg: ClawdbotConfig = { commands: { useAccessGroups: true }, channels: { @@ -665,18 +679,8 @@ describe("handleFeishuMessage command authorization", () => { await dispatchMessage({ cfg, event }); - expect(mockResolveCommandAuthorizedFromAuthorizers).toHaveBeenCalledWith({ - useAccessGroups: true, - authorizers: [{ configured: true, allowed: false }], - }); - expect(mockFinalizeInboundContext).toHaveBeenCalledTimes(1); - expect(mockFinalizeInboundContext).toHaveBeenCalledWith( - expect.objectContaining({ - CommandAuthorized: false, - SenderId: "ou-attacker", - Surface: "feishu", - }), - ); + expect(mockResolveCommandAuthorizedFromAuthorizers).not.toHaveBeenCalled(); + expect(mockFinalizeInboundContext).not.toHaveBeenCalled(); }); it("reads pairing allow store for non-command DMs when dmPolicy is pairing", async () => { @@ -1610,7 +1614,11 @@ describe("handleFeishuMessage command authorization", () => { MediaTypes: ["audio/ogg"], ChatType: "direct", }, - cfg, + cfg: expect.objectContaining({ + channels: expect.objectContaining({ + feishu: expect.objectContaining({ dmPolicy: "open" }), + }), + }), }); expect(mockFinalizeInboundContext).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/extensions/feishu/src/bot.ts b/extensions/feishu/src/bot.ts index 11c1ab08014..9d734fbafd7 100644 --- a/extensions/feishu/src/bot.ts +++ b/extensions/feishu/src/bot.ts @@ -17,6 +17,7 @@ import { resolveOpenProviderRuntimeGroupPolicy, warnMissingProviderGroupPolicyFallbackOnce, } from "openclaw/plugin-sdk/runtime-group-policy"; +import { resolveOpenDmAllowlistAccess } from "openclaw/plugin-sdk/security-runtime"; import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime"; import { resolveFeishuRuntimeAccount } from "./accounts.js"; import { @@ -641,9 +642,7 @@ export async function handleFeishuMessage(params: { cfg, ); const storeAllowFrom = - !isGroup && - dmPolicy !== "allowlist" && - (dmPolicy !== "open" || shouldComputeCommandAuthorized) + !isGroup && dmPolicy !== "allowlist" && dmPolicy !== "open" ? await pairing.readAllowFromStore().catch(() => []) : []; const effectiveDmAllowFrom = [...configAllowFrom, ...storeAllowFrom]; @@ -654,7 +653,21 @@ export async function handleFeishuMessage(params: { senderName: ctx.senderName, }).allowed; - if (isDirect && dmPolicy !== "open" && !dmAllowed) { + const dmAccessAllowed = + dmPolicy === "open" + ? resolveOpenDmAllowlistAccess({ + effectiveAllowFrom: effectiveDmAllowFrom, + isSenderAllowed: (allowFrom) => + resolveFeishuAllowlistMatch({ + allowFrom, + senderId: ctx.senderOpenId, + senderIds: [senderUserId], + senderName: ctx.senderName, + }).allowed, + }).decision === "allow" + : dmAllowed; + + if (isDirect && !dmAccessAllowed) { if (dmPolicy === "pairing") { await pairing.issueChallenge({ senderId: ctx.senderOpenId, diff --git a/extensions/feishu/src/comment-handler.test.ts b/extensions/feishu/src/comment-handler.test.ts index 396ee66c215..ad7a241f3df 100644 --- a/extensions/feishu/src/comment-handler.test.ts +++ b/extensions/feishu/src/comment-handler.test.ts @@ -35,6 +35,7 @@ function buildConfig(overrides?: Partial): ClawdbotConfig { feishu: { enabled: true, dmPolicy: "open", + allowFrom: ["*"], }, }, ...overrides, diff --git a/extensions/feishu/src/comment-handler.ts b/extensions/feishu/src/comment-handler.ts index 59c8c3c64b8..522df8ac573 100644 --- a/extensions/feishu/src/comment-handler.ts +++ b/extensions/feishu/src/comment-handler.ts @@ -1,4 +1,5 @@ import type { ResolvedAgentRoute } from "openclaw/plugin-sdk/routing"; +import { resolveOpenDmAllowlistAccess } from "openclaw/plugin-sdk/security-runtime"; import { resolveFeishuRuntimeAccount } from "./accounts.js"; import { createFeishuClient } from "./client.js"; import { createFeishuCommentReplyDispatcher } from "./comment-dispatcher.js"; @@ -96,7 +97,19 @@ export async function handleFeishuCommentEvent( senderId: turn.senderId, senderIds: [turn.senderUserId], }).allowed; - if (dmPolicy !== "open" && !senderAllowed) { + const dmAccessAllowed = + dmPolicy === "open" + ? resolveOpenDmAllowlistAccess({ + effectiveAllowFrom: effectiveDmAllowFrom, + isSenderAllowed: (allowFrom) => + resolveFeishuAllowlistMatch({ + allowFrom, + senderId: turn.senderId, + senderIds: [turn.senderUserId], + }).allowed, + }).decision === "allow" + : senderAllowed; + if (!dmAccessAllowed) { if (dmPolicy === "pairing") { const client = createFeishuClient(account); await pairing.issueChallenge({ diff --git a/extensions/googlechat/src/monitor-access.ts b/extensions/googlechat/src/monitor-access.ts index 20abe76bccb..6d44ec7345c 100644 --- a/extensions/googlechat/src/monitor-access.ts +++ b/extensions/googlechat/src/monitor-access.ts @@ -254,7 +254,7 @@ export async function applyGoogleChatInboundAccessPolicy(params: { }); const shouldComputeAuth = core.channel.commands.shouldComputeCommandAuthorized(rawBody, config); const storeAllowFrom = - !isGroup && dmPolicy !== "allowlist" && (dmPolicy !== "open" || shouldComputeAuth) + !isGroup && dmPolicy !== "allowlist" && dmPolicy !== "open" ? await pairing.readAllowFromStore().catch(() => []) : []; const access = resolveDmGroupAccessWithLists({ diff --git a/extensions/imessage/src/monitor/inbound-processing.test.ts b/extensions/imessage/src/monitor/inbound-processing.test.ts index 52afd426942..b58b17ff4e6 100644 --- a/extensions/imessage/src/monitor/inbound-processing.test.ts +++ b/extensions/imessage/src/monitor/inbound-processing.test.ts @@ -31,7 +31,7 @@ describe("resolveIMessageInboundDecision echo detection", () => { cfg, accountId: "default", opts: undefined, - allowFrom: [], + allowFrom: ["*"], groupAllowFrom: [], groupPolicy: "open", dmPolicy: "open", @@ -303,7 +303,12 @@ describe("describeIMessageEchoDropLog", () => { describe("resolveIMessageInboundDecision command auth", () => { const cfg = {} as OpenClawConfig; - const resolveDmCommandDecision = (params: { messageId: number; storeAllowFrom: string[] }) => + const resolveDmCommandDecision = (params: { + messageId: number; + storeAllowFrom: string[]; + dmPolicy?: "open" | "pairing" | "allowlist" | "disabled"; + allowFrom?: string[]; + }) => resolveIMessageInboundDecision({ cfg, accountId: "default", @@ -317,10 +322,10 @@ describe("resolveIMessageInboundDecision command auth", () => { opts: undefined, messageText: "/status", bodyText: "/status", - allowFrom: [], + allowFrom: params.allowFrom ?? [], groupAllowFrom: [], groupPolicy: "open", - dmPolicy: "open", + dmPolicy: params.dmPolicy ?? "open", storeAllowFrom: params.storeAllowFrom, historyLimit: 0, groupHistories: new Map(), @@ -334,16 +339,13 @@ describe("resolveIMessageInboundDecision command auth", () => { storeAllowFrom: [], }); - expect(decision.kind).toBe("dispatch"); - if (decision.kind !== "dispatch") { - return; - } - expect(decision.commandAuthorized).toBe(false); + expect(decision).toEqual({ kind: "drop", reason: "dmPolicy blocked" }); }); - it("authorizes DM commands for senders in pairing-store allowlist", () => { + it("authorizes DM commands for senders in pairing-mode store allowlist", () => { const decision = resolveDmCommandDecision({ messageId: 101, + dmPolicy: "pairing", storeAllowFrom: ["+15555550123"], }); diff --git a/extensions/irc/src/inbound.ts b/extensions/irc/src/inbound.ts index 17309b6cdb3..595e060076f 100644 --- a/extensions/irc/src/inbound.ts +++ b/extensions/irc/src/inbound.ts @@ -207,36 +207,34 @@ export async function handleIrcInbound(params: { runtime.log?.(`irc: drop DM sender=${senderDisplay} (dmPolicy=disabled)`); return; } - if (dmPolicy !== "open") { - const dmAllowed = resolveIrcAllowlistMatch({ - allowFrom: effectiveAllowFrom, - message, - allowNameMatching, - }).allowed; - if (!dmAllowed) { - if (dmPolicy === "pairing") { - await pairing.issueChallenge({ - senderId: normalizeLowercaseStringOrEmpty(senderDisplay), - senderIdLine: `Your IRC id: ${senderDisplay}`, - meta: { name: message.senderNick || undefined }, - sendPairingReply: async (text) => { - await deliverIrcReply({ - payload: { text }, - cfg: config, - target: message.senderNick, - accountId: account.accountId, - sendReply: params.sendReply, - statusSink, - }); - }, - onReplyError: (err) => { - runtime.error?.(`irc: pairing reply failed for ${senderDisplay}: ${String(err)}`); - }, - }); - } - runtime.log?.(`irc: drop DM sender ${senderDisplay} (dmPolicy=${dmPolicy})`); - return; + const dmAllowed = resolveIrcAllowlistMatch({ + allowFrom: effectiveAllowFrom, + message, + allowNameMatching, + }).allowed; + if (!dmAllowed) { + if (dmPolicy === "pairing") { + await pairing.issueChallenge({ + senderId: normalizeLowercaseStringOrEmpty(senderDisplay), + senderIdLine: `Your IRC id: ${senderDisplay}`, + meta: { name: message.senderNick || undefined }, + sendPairingReply: async (text) => { + await deliverIrcReply({ + payload: { text }, + cfg: config, + target: message.senderNick, + accountId: account.accountId, + sendReply: params.sendReply, + statusSink, + }); + }, + onReplyError: (err) => { + runtime.error?.(`irc: pairing reply failed for ${senderDisplay}: ${String(err)}`); + }, + }); } + runtime.log?.(`irc: drop DM sender ${senderDisplay} (dmPolicy=${dmPolicy})`); + return; } } diff --git a/extensions/line/src/bot-handlers.test.ts b/extensions/line/src/bot-handlers.test.ts index 15d65e2e83e..b14ce78d256 100644 --- a/extensions/line/src/bot-handlers.test.ts +++ b/extensions/line/src/bot-handlers.test.ts @@ -292,6 +292,7 @@ function createLineWebhookTestContext(params: { const lineConfig = { ...(params.groupPolicy ? { groupPolicy: params.groupPolicy } : {}), ...(params.dmPolicy ? { dmPolicy: params.dmPolicy } : {}), + ...(params.dmPolicy === "open" ? { allowFrom: ["*"] } : {}), }; return { cfg: { channels: { line: lineConfig } }, @@ -830,14 +831,14 @@ describe("handleLineWebhookEvents", () => { } as PostbackEvent; const context: Parameters[1] = { - cfg: { channels: { line: { dmPolicy: "open" } } }, + cfg: { channels: { line: { dmPolicy: "open", allowFrom: ["*"] } } }, account: { accountId: "default", enabled: true, channelAccessToken: "token", channelSecret: "secret", tokenSource: "config", - config: { dmPolicy: "open" }, + config: { dmPolicy: "open", allowFrom: ["*"] }, }, runtime: createRuntime(), mediaMaxBytes: 1, diff --git a/extensions/line/src/bot-handlers.ts b/extensions/line/src/bot-handlers.ts index e0105cd9b89..c59bf177f16 100644 --- a/extensions/line/src/bot-handlers.ts +++ b/extensions/line/src/bot-handlers.ts @@ -335,7 +335,7 @@ async function shouldProcessLineEvent( return denied; } - const dmAllowed = dmPolicy === "open" || isSenderAllowed({ allow: effectiveDmAllow, senderId }); + const dmAllowed = isSenderAllowed({ allow: effectiveDmAllow, senderId }); if (!dmAllowed) { if (dmPolicy === "pairing") { if (!senderId) { diff --git a/extensions/matrix/src/matrix/monitor/access-state.test.ts b/extensions/matrix/src/matrix/monitor/access-state.test.ts index f938e97a1b8..e5cbc8d2af7 100644 --- a/extensions/matrix/src/matrix/monitor/access-state.test.ts +++ b/extensions/matrix/src/matrix/monitor/access-state.test.ts @@ -47,6 +47,21 @@ describe("resolveMatrixMonitorAccessState", () => { ]); }); + it("does not let pairing-store entries authorize open DMs without wildcard", () => { + const state = resolveMatrixMonitorAccessState({ + allowFrom: [], + storeAllowFrom: ["@alice:example.org"], + dmPolicy: "open", + groupAllowFrom: [], + roomUsers: [], + senderId: "@alice:example.org", + isRoom: false, + }); + + expect(state.effectiveAllowFrom).toEqual([]); + expect(state.directAllowMatch.allowed).toBe(false); + }); + it("does not let configured DM allowFrom authorize room control commands", () => { const state = resolveMatrixMonitorAccessState({ allowFrom: ["@owner:example.org"], diff --git a/extensions/matrix/src/matrix/monitor/access-state.ts b/extensions/matrix/src/matrix/monitor/access-state.ts index 0733778de96..f6bc41171e7 100644 --- a/extensions/matrix/src/matrix/monitor/access-state.ts +++ b/extensions/matrix/src/matrix/monitor/access-state.ts @@ -1,3 +1,4 @@ +import { mergeDmAllowFromSources } from "openclaw/plugin-sdk/allow-from"; import { normalizeMatrixAllowList, resolveMatrixAllowListMatch } from "./allowlist.js"; type MatrixCommandAuthorizer = { @@ -25,16 +26,20 @@ export type MatrixMonitorAccessState = { export function resolveMatrixMonitorAccessState(params: { allowFrom: Array; storeAllowFrom: Array; + dmPolicy?: "open" | "pairing" | "allowlist" | "disabled"; groupAllowFrom: Array; roomUsers: Array; senderId: string; isRoom: boolean; }): MatrixMonitorAccessState { const configuredAllowFrom = normalizeMatrixAllowList(params.allowFrom); - const effectiveAllowFrom = normalizeMatrixAllowList([ - ...configuredAllowFrom, - ...params.storeAllowFrom, - ]); + const effectiveAllowFrom = normalizeMatrixAllowList( + mergeDmAllowFromSources({ + allowFrom: configuredAllowFrom, + storeAllowFrom: params.storeAllowFrom, + dmPolicy: params.dmPolicy, + }), + ); const effectiveGroupAllowFrom = normalizeMatrixAllowList(params.groupAllowFrom); const effectiveRoomUsers = normalizeMatrixAllowList(params.roomUsers); const commandAllowFrom = params.isRoom ? [] : effectiveAllowFrom; diff --git a/extensions/matrix/src/matrix/monitor/events.test.ts b/extensions/matrix/src/matrix/monitor/events.test.ts index 998f2481600..5194c6f593b 100644 --- a/extensions/matrix/src/matrix/monitor/events.test.ts +++ b/extensions/matrix/src/matrix/monitor/events.test.ts @@ -148,6 +148,9 @@ function createHarness(params?: { }), } as unknown as MatrixClient; + const dmPolicy = params?.dmPolicy ?? "open"; + const allowFrom = params?.allowFrom ?? (dmPolicy === "open" ? ["*"] : []); + registerMatrixMonitorEvents({ cfg: params?.cfg ?? { channels: { matrix: {} } }, client, @@ -155,9 +158,9 @@ function createHarness(params?: { accountId: params?.accountId ?? "default", encryption: params?.authEncryption ?? true, } as MatrixAuth, - allowFrom: params?.allowFrom ?? [], + allowFrom, dmEnabled: params?.dmEnabled ?? true, - dmPolicy: params?.dmPolicy ?? "open", + dmPolicy, readStoreAllowFrom, directTracker: { invalidateRoom, diff --git a/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts b/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts index 78d58fac0a9..8855894aacd 100644 --- a/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts +++ b/extensions/matrix/src/matrix/monitor/handler.test-helpers.ts @@ -119,7 +119,19 @@ export function createMatrixHandlerTestHarness( counts: { final: 0, block: 0, tool: 0 }, })); const enqueueSystemEvent = options.enqueueSystemEvent ?? vi.fn(); - const cfgForHandler = options.cfg ?? {}; + const dmPolicy = options.dmPolicy ?? "open"; + const allowFrom = options.allowFrom ?? (dmPolicy === "open" ? ["*"] : []); + const cfgForHandler = + options.cfg ?? + ({ + channels: { + matrix: { + dm: { + allowFrom, + }, + }, + }, + } as const); const handler = createMatrixRoomMessageHandler({ client: { @@ -216,7 +228,7 @@ export function createMatrixHandlerTestHarness( error: () => {}, } as RuntimeLogger), logVerboseMessage: options.logVerboseMessage ?? (() => {}), - allowFrom: options.allowFrom ?? [], + allowFrom, allowFromResolvedEntries: options.allowFromResolvedEntries, groupAllowFrom: options.groupAllowFrom ?? [], groupAllowFromResolvedEntries: options.groupAllowFromResolvedEntries, @@ -232,7 +244,7 @@ export function createMatrixHandlerTestHarness( previewToolProgressEnabled: options.previewToolProgressEnabled ?? false, blockStreamingEnabled: options.blockStreamingEnabled ?? false, dmEnabled: options.dmEnabled ?? true, - dmPolicy: options.dmPolicy ?? "open", + dmPolicy, textLimit: options.textLimit ?? 8_000, mediaMaxBytes: options.mediaMaxBytes ?? 10_000_000, startupMs: options.startupMs ?? 0, diff --git a/extensions/matrix/src/matrix/monitor/handler.test.ts b/extensions/matrix/src/matrix/monitor/handler.test.ts index c2cc482024d..298df270aa1 100644 --- a/extensions/matrix/src/matrix/monitor/handler.test.ts +++ b/extensions/matrix/src/matrix/monitor/handler.test.ts @@ -1614,7 +1614,7 @@ describe("matrix monitor handler pairing account scope", () => { channels: { matrix: { threadReplies: "always", - dm: { threadReplies: "off" }, + dm: { allowFrom: ["*"], threadReplies: "off" }, }, }, }, diff --git a/extensions/matrix/src/matrix/monitor/handler.ts b/extensions/matrix/src/matrix/monitor/handler.ts index c58a1be67a6..2282be8885f 100644 --- a/extensions/matrix/src/matrix/monitor/handler.ts +++ b/extensions/matrix/src/matrix/monitor/handler.ts @@ -701,7 +701,10 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam senderNamePromise ??= getMemberDisplayName(roomId, senderId).catch(() => senderId); return await senderNamePromise; }; - const storeAllowFrom = isDirectMessage ? await readStoreAllowFrom() : []; + const storeAllowFrom = + isDirectMessage && dmPolicy !== "allowlist" && dmPolicy !== "open" + ? await readStoreAllowFrom() + : []; const roomUsers = roomConfig?.users ?? []; const liveCfg = core.config.current() as CoreConfig; const liveAccountAllowlists = resolveMatrixAccountAllowlistConfig({ @@ -729,6 +732,7 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam const accessState = resolveMatrixMonitorAccessState({ allowFrom: liveDmAllowFrom, storeAllowFrom, + dmPolicy, groupAllowFrom: liveGroupAllowFrom, roomUsers, senderId, @@ -749,63 +753,59 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam await commitInboundEventIfClaimed(); return undefined; } - if (dmPolicy !== "open") { - const allowMatchMeta = formatAllowlistMatchMeta(directAllowMatch); - if (!directAllowMatch.allowed) { - if (!isReactionEvent && dmPolicy === "pairing") { - const senderName = await getSenderName(); - const { code, created } = await core.channel.pairing.upsertPairingRequest({ + const allowMatchMeta = formatAllowlistMatchMeta(directAllowMatch); + if (!directAllowMatch.allowed) { + if (!isReactionEvent && dmPolicy === "pairing") { + const senderName = await getSenderName(); + const { code, created } = await core.channel.pairing.upsertPairingRequest({ + channel: "matrix", + id: senderId, + accountId, + meta: { name: senderName }, + }); + if (shouldSendPairingReply(senderId, created)) { + const pairingReply = core.channel.pairing.buildPairingReply({ channel: "matrix", - id: senderId, - accountId, - meta: { name: senderName }, + idLine: `Your Matrix user id: ${senderId}`, + code, }); - if (shouldSendPairingReply(senderId, created)) { - const pairingReply = core.channel.pairing.buildPairingReply({ - channel: "matrix", - idLine: `Your Matrix user id: ${senderId}`, - code, - }); - logVerboseMessage( + logVerboseMessage( + created + ? `matrix pairing request sender=${senderId} name=${senderName ?? "unknown"} (${allowMatchMeta})` + : `matrix pairing reminder sender=${senderId} name=${senderName ?? "unknown"} (${allowMatchMeta})`, + ); + try { + const { sendMessageMatrix } = await loadMatrixSendModule(); + await sendMessageMatrix( + `room:${roomId}`, created - ? `matrix pairing request sender=${senderId} name=${senderName ?? "unknown"} (${allowMatchMeta})` - : `matrix pairing reminder sender=${senderId} name=${senderName ?? "unknown"} (${allowMatchMeta})`, - ); - try { - const { sendMessageMatrix } = await loadMatrixSendModule(); - await sendMessageMatrix( - `room:${roomId}`, - created - ? pairingReply - : `${pairingReply}\n\nPairing request is still pending approval. Reusing existing code.`, - { - client, - cfg, - accountId, - }, - ); - await commitInboundEventIfClaimed(); - } catch (err) { - logVerboseMessage( - `matrix pairing reply failed for ${senderId}: ${String(err)}`, - ); - return undefined; - } - } else { - logVerboseMessage( - `matrix pairing reminder suppressed sender=${senderId} (cooldown)`, + ? pairingReply + : `${pairingReply}\n\nPairing request is still pending approval. Reusing existing code.`, + { + client, + cfg, + accountId, + }, ); await commitInboundEventIfClaimed(); + } catch (err) { + logVerboseMessage(`matrix pairing reply failed for ${senderId}: ${String(err)}`); + return undefined; } - } - if (isReactionEvent || dmPolicy !== "pairing") { + } else { logVerboseMessage( - `matrix: blocked ${isReactionEvent ? "reaction" : "dm"} sender ${senderId} (dmPolicy=${dmPolicy}, ${allowMatchMeta})`, + `matrix pairing reminder suppressed sender=${senderId} (cooldown)`, ); await commitInboundEventIfClaimed(); } - return undefined; } + if (isReactionEvent || dmPolicy !== "pairing") { + logVerboseMessage( + `matrix: blocked ${isReactionEvent ? "reaction" : "dm"} sender ${senderId} (dmPolicy=${dmPolicy}, ${allowMatchMeta})`, + ); + await commitInboundEventIfClaimed(); + } + return undefined; } } diff --git a/extensions/matrix/src/matrix/monitor/verification-events.ts b/extensions/matrix/src/matrix/monitor/verification-events.ts index 44a4cd86c17..b759a307b2d 100644 --- a/extensions/matrix/src/matrix/monitor/verification-events.ts +++ b/extensions/matrix/src/matrix/monitor/verification-events.ts @@ -358,13 +358,14 @@ async function isVerificationNoticeAuthorized(params: { ); return false; } - if (params.dmPolicy === "open") { - return true; - } - const storeAllowFrom = await params.readStoreAllowFrom(); + const storeAllowFrom = + params.dmPolicy !== "allowlist" && params.dmPolicy !== "open" + ? await params.readStoreAllowFrom() + : []; const accessState = resolveMatrixMonitorAccessState({ allowFrom: params.allowFrom, storeAllowFrom, + dmPolicy: params.dmPolicy, // Verification flows only exist in strict DMs, so room/group allowlists do // not participate in the authorization decision here. groupAllowFrom: [], diff --git a/extensions/mattermost/src/mattermost/monitor-auth.test.ts b/extensions/mattermost/src/mattermost/monitor-auth.test.ts index 471d5b42b27..e36c8e90133 100644 --- a/extensions/mattermost/src/mattermost/monitor-auth.test.ts +++ b/extensions/mattermost/src/mattermost/monitor-auth.test.ts @@ -89,7 +89,7 @@ describe("mattermost monitor auth", () => { }); }); - it("authorizes direct messages in open mode and blocks disabled/group-restricted channels", async () => { + it("requires open direct messages to match the effective allowlist", async () => { isDangerousNameMatchingEnabled.mockReturnValue(false); resolveEffectiveAllowFromLists.mockReturnValue({ effectiveAllowFrom: [], @@ -118,11 +118,35 @@ describe("mattermost monitor auth", () => { allowTextCommands: false, hasControlCommand: false, }), + ).toMatchObject({ + ok: false, + denyReason: "unauthorized", + kind: "direct", + }); + + resolveEffectiveAllowFromLists.mockReturnValue({ + effectiveAllowFrom: ["*"], + effectiveGroupAllowFrom: [], + }); + resolveAllowlistMatchSimple.mockReturnValue({ allowed: true }); + + expect( + authorizeMattermostCommandInvocation({ + account: { + config: { dmPolicy: "open", allowFrom: ["*"] }, + } as never, + cfg: {} as never, + senderId: "alice", + senderName: "Alice", + channelId: "dm-1", + channelInfo: { type: "D", name: "alice", display_name: "Alice" } as never, + allowTextCommands: false, + hasControlCommand: false, + }), ).toMatchObject({ ok: true, commandAuthorized: true, kind: "direct", - roomLabel: "#alice", }); expect( diff --git a/extensions/mattermost/src/mattermost/monitor-auth.ts b/extensions/mattermost/src/mattermost/monitor-auth.ts index 33d77ebe5ca..912d96dbcb0 100644 --- a/extensions/mattermost/src/mattermost/monitor-auth.ts +++ b/extensions/mattermost/src/mattermost/monitor-auth.ts @@ -202,9 +202,7 @@ export function authorizeMattermostCommandInvocation(params: { }); const commandAuthorized = - kind === "direct" - ? dmPolicy === "open" || senderAllowedForCommands - : commandGate.commandAuthorized; + kind === "direct" ? senderAllowedForCommands : commandGate.commandAuthorized; if (kind === "direct") { if (dmPolicy === "disabled") { @@ -221,7 +219,7 @@ export function authorizeMattermostCommandInvocation(params: { }; } - if (dmPolicy !== "open" && !senderAllowedForCommands) { + if (!senderAllowedForCommands) { return { ok: false, denyReason: dmPolicy === "pairing" ? "dm-pairing" : "unauthorized", diff --git a/extensions/qqbot/src/engine/access/access-control.test.ts b/extensions/qqbot/src/engine/access/access-control.test.ts index 9fd7f7db9c5..4b6111bfb11 100644 --- a/extensions/qqbot/src/engine/access/access-control.test.ts +++ b/extensions/qqbot/src/engine/access/access-control.test.ts @@ -4,12 +4,27 @@ import { QQBOT_ACCESS_REASON } from "./types.js"; describe("resolveQQBotAccess", () => { describe("DM scenarios", () => { - it("allows everyone when no allowFrom is configured (open)", () => { + it("allows default-open DMs when allowFrom is omitted", () => { const result = resolveQQBotAccess({ isGroup: false, senderId: "USER1" }); expect(result).toMatchObject({ decision: "allow", reasonCode: QQBOT_ACCESS_REASON.DM_POLICY_OPEN, dmPolicy: "open", + effectiveAllowFrom: ["*"], + }); + }); + + it("allows default-open DMs when allowFrom is explicitly empty", () => { + const result = resolveQQBotAccess({ + isGroup: false, + senderId: "USER1", + allowFrom: [], + }); + expect(result).toMatchObject({ + decision: "allow", + reasonCode: QQBOT_ACCESS_REASON.DM_POLICY_OPEN, + dmPolicy: "open", + effectiveAllowFrom: ["*"], }); }); @@ -34,6 +49,18 @@ describe("resolveQQBotAccess", () => { expect(result.dmPolicy).toBe("allowlist"); }); + it("allows open mode when sender matches restrictive allowFrom", () => { + const result = resolveQQBotAccess({ + isGroup: false, + senderId: "USER1", + allowFrom: ["USER1"], + dmPolicy: "open", + }); + expect(result.decision).toBe("allow"); + expect(result.reasonCode).toBe(QQBOT_ACCESS_REASON.DM_POLICY_ALLOWLISTED); + expect(result.reason).toBe("dmPolicy=open (allowlisted)"); + }); + it("blocks sender not in allowlist", () => { const result = resolveQQBotAccess({ isGroup: false, diff --git a/extensions/qqbot/src/engine/access/access-control.ts b/extensions/qqbot/src/engine/access/access-control.ts index 0997e1e61a1..dad805e54c4 100644 --- a/extensions/qqbot/src/engine/access/access-control.ts +++ b/extensions/qqbot/src/engine/access/access-control.ts @@ -43,7 +43,7 @@ export interface QQBotAccessInput extends EffectivePolicyInput { * - otherwise → allow * - Direct message: * - `dmPolicy=disabled` → block - * - `dmPolicy=open` → allow + * - `dmPolicy=open` → allow wildcard, legacy empty allowFrom, or matching allowFrom * - `dmPolicy=allowlist`: * - empty effectiveAllowFrom → block (empty_allowlist) * - sender not in list → block (not_allowlisted) @@ -63,7 +63,9 @@ export function resolveQQBotAccess(input: QQBotAccessInput): QQBotAccessResult { ? input.groupAllowFrom : (input.allowFrom ?? []); - const effectiveAllowFrom = normalizeQQBotAllowFrom(input.allowFrom); + const normalizedAllowFrom = normalizeQQBotAllowFrom(input.allowFrom); + const effectiveAllowFrom = + dmPolicy === "open" && normalizedAllowFrom.length === 0 ? ["*"] : normalizedAllowFrom; const effectiveGroupAllowFrom = normalizeQQBotAllowFrom(rawGroupAllowFrom); const isSenderAllowed = createQQBotSenderMatcher(input.senderId); @@ -158,11 +160,27 @@ function evaluateDmDecision(ctx: DecisionContext): QQBotAccessResult { } if (ctx.dmPolicy === "open") { + if (ctx.effectiveAllowFrom.includes("*")) { + return { + ...base, + decision: "allow", + reasonCode: QQBOT_ACCESS_REASON.DM_POLICY_OPEN, + reason: "dmPolicy=open", + }; + } + if (ctx.isSenderAllowed(ctx.effectiveAllowFrom)) { + return { + ...base, + decision: "allow", + reasonCode: QQBOT_ACCESS_REASON.DM_POLICY_ALLOWLISTED, + reason: "dmPolicy=open (allowlisted)", + }; + } return { ...base, - decision: "allow", - reasonCode: QQBOT_ACCESS_REASON.DM_POLICY_OPEN, - reason: "dmPolicy=open", + decision: "block", + reasonCode: QQBOT_ACCESS_REASON.DM_POLICY_NOT_ALLOWLISTED, + reason: "dmPolicy=open (not allowlisted)", }; } diff --git a/extensions/slack/src/monitor/auth.ts b/extensions/slack/src/monitor/auth.ts index 5040e2e7057..4bcf169af43 100644 --- a/extensions/slack/src/monitor/auth.ts +++ b/extensions/slack/src/monitor/auth.ts @@ -257,22 +257,20 @@ export async function authorizeSlackSystemEventSender(params: { if (!params.ctx.dmEnabled || params.ctx.dmPolicy === "disabled") { return { allowed: false, reason: "dm-disabled", channelType, channelName }; } - if (params.ctx.dmPolicy !== "open") { - const allowFromLower = await resolveAllowFromLower(true); - const senderAllowListed = isSlackSenderAllowListed({ - allowListLower: allowFromLower, - senderId, - senderName, - allowNameMatching: params.ctx.allowNameMatching, - }); - if (!senderAllowListed) { - return { - allowed: false, - reason: "sender-not-allowlisted", - channelType, - channelName, - }; - } + const allowFromLower = await resolveAllowFromLower(true); + const senderAllowListed = isSlackSenderAllowListed({ + allowListLower: allowFromLower, + senderId, + senderName, + allowNameMatching: params.ctx.allowNameMatching, + }); + if (!senderAllowListed) { + return { + allowed: false, + reason: "sender-not-allowlisted", + channelType, + channelName, + }; } } else if (!channelId) { // No channel context. Preserve the existing open default unless a global diff --git a/extensions/slack/src/monitor/dm-auth.ts b/extensions/slack/src/monitor/dm-auth.ts index fa360d5cfde..7b16f9098ec 100644 --- a/extensions/slack/src/monitor/dm-auth.ts +++ b/extensions/slack/src/monitor/dm-auth.ts @@ -20,9 +20,6 @@ export async function authorizeSlackDirectMessage(params: { await params.onDisabled(); return false; } - if (params.ctx.dmPolicy === "open") { - return true; - } const sender = await params.resolveSenderName(params.senderId); const senderName = sender?.name ?? undefined; diff --git a/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts b/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts index 5961fc84c87..816fac3b5e9 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test-helpers.ts @@ -31,7 +31,7 @@ export function createInboundSlackTestContext(params: { mainKey: "main", dmEnabled: true, dmPolicy: "open", - allowFrom: [], + allowFrom: ["*"], allowNameMatching: false, groupDmEnabled: true, groupDmChannels: [], diff --git a/extensions/slack/src/monitor/message-handler/prepare.test.ts b/extensions/slack/src/monitor/message-handler/prepare.test.ts index dce6ff36d55..be910e77328 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.test.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.test.ts @@ -148,6 +148,7 @@ describe("slack prepareSlackMessage inbound contract", () => { followUpTs: string; currentTs: string; channelsConfig?: Parameters[0]["channelsConfig"]; + allowFrom?: string[]; resolveChannelName?: (channelId: string) => Promise<{ name?: string; type?: SlackMessageEvent["channel_type"]; @@ -189,7 +190,7 @@ describe("slack prepareSlackMessage inbound contract", () => { replyToMode: "all", channelsConfig: params.channelsConfig, }); - ctx.allowFrom = ["u-owner"]; + ctx.allowFrom = params.allowFrom ?? ["u-owner"]; ctx.resolveUserName = async (id: string) => ({ name: id === params.user ? params.userName : "Owner", }); @@ -680,6 +681,7 @@ describe("slack prepareSlackMessage inbound contract", () => { replyTs: "300.500", followUpTs: "300.800", currentTs: "301.000", + allowFrom: ["*"], }); expectThreadContextAllowsHumanHistory( diff --git a/extensions/slack/src/monitor/message-handler/prepare.ts b/extensions/slack/src/monitor/message-handler/prepare.ts index 6b60e9a7db6..d1f50c5a1c5 100644 --- a/extensions/slack/src/monitor/message-handler/prepare.ts +++ b/extensions/slack/src/monitor/message-handler/prepare.ts @@ -416,9 +416,7 @@ export async function prepareSlackMessage(params: { ? normalizeAllowListLower(channelConfig?.users) : [] : isDirectMessage - ? ctx.dmPolicy === "open" - ? [] - : allowFromLower + ? allowFromLower : []; const contextVisibilityMode = resolveChannelContextVisibilityMode({ cfg: ctx.cfg, diff --git a/extensions/slack/src/monitor/slash.test.ts b/extensions/slack/src/monitor/slash.test.ts index b6dda65d3ae..6b42160e79e 100644 --- a/extensions/slack/src/monitor/slash.test.ts +++ b/extensions/slack/src/monitor/slash.test.ts @@ -976,7 +976,7 @@ describe("slack slash commands access groups", () => { it("still treats D-prefixed channel ids as DMs when lookup fails", async () => { const harness = createPolicyHarness({ - allowFrom: [], + allowFrom: ["*"], channelId: "D123", channelName: "notdirectmessage", resolveChannelName: async () => ({}), @@ -996,12 +996,12 @@ describe("slack slash commands access groups", () => { const dispatchArg = dispatchMock.mock.calls[0]?.[0] as { ctx?: { CommandAuthorized?: boolean }; }; - expect(dispatchArg?.ctx?.CommandAuthorized).toBe(false); + expect(dispatchArg?.ctx?.CommandAuthorized).toBe(true); }); it("computes CommandAuthorized for DM slash commands when dmPolicy is open", async () => { const harness = createPolicyHarness({ - allowFrom: ["U_OWNER"], + allowFrom: ["*"], channelId: "D999", channelName: "directmessage", resolveChannelName: async () => ({ name: "directmessage", type: "im" }), @@ -1020,7 +1020,7 @@ describe("slack slash commands access groups", () => { const dispatchArg = dispatchMock.mock.calls[0]?.[0] as { ctx?: { CommandAuthorized?: boolean }; }; - expect(dispatchArg?.ctx?.CommandAuthorized).toBe(false); + expect(dispatchArg?.ctx?.CommandAuthorized).toBe(true); }); it("classifies MPIM slash commands as group chat context", async () => { diff --git a/extensions/synology-chat/src/channel.integration.test.ts b/extensions/synology-chat/src/channel.integration.test.ts index 1896b79d97c..a8db3045386 100644 --- a/extensions/synology-chat/src/channel.integration.test.ts +++ b/extensions/synology-chat/src/channel.integration.test.ts @@ -109,6 +109,7 @@ describe("Synology channel wiring integration", () => { incomingUrl: "https://nas.example.com/incoming-alpha", webhookPath: "/webhook/synology-alpha", dmPolicy: "open", + allowedUserIds: ["*"], }, beta: { enabled: true, @@ -116,6 +117,7 @@ describe("Synology channel wiring integration", () => { incomingUrl: "https://nas.example.com/incoming-beta", webhookPath: "/webhook/synology-beta", dmPolicy: "open", + allowedUserIds: ["*"], }, }, }, diff --git a/extensions/synology-chat/src/channel.test.ts b/extensions/synology-chat/src/channel.test.ts index e24876d7cf1..12ec98cc802 100644 --- a/extensions/synology-chat/src/channel.test.ts +++ b/extensions/synology-chat/src/channel.test.ts @@ -284,11 +284,18 @@ describe("createSynologyChatPlugin", () => { it("warns when dmPolicy is open", () => { const plugin = createSynologyChatPlugin(); - const account = makeSecurityAccount({ dmPolicy: "open" }); + const account = makeSecurityAccount({ dmPolicy: "open", allowedUserIds: ["*"] }); const warnings = plugin.security.collectWarnings({ cfg: {}, account }); expect(warnings.some((w: string) => w.includes("open"))).toBe(true); }); + it("warns when dmPolicy is open and allowedUserIds is empty", () => { + const plugin = createSynologyChatPlugin(); + const account = makeSecurityAccount({ dmPolicy: "open", allowedUserIds: [] }); + const warnings = plugin.security.collectWarnings({ cfg: {}, account }); + expect(warnings.some((w: string) => w.includes("empty allowedUserIds"))).toBe(true); + }); + it("warns when dmPolicy is allowlist and allowedUserIds is empty", () => { const plugin = createSynologyChatPlugin(); const account = makeSecurityAccount(); @@ -531,6 +538,26 @@ describe("createSynologyChatPlugin", () => { expect(registerMock).not.toHaveBeenCalled(); }); + it("startAccount refuses open accounts with empty allowedUserIds", async () => { + const registerMock = registerSynologyWebhookRouteMock; + registerMock.mockClear(); + const plugin = createSynologyChatPlugin(); + const { ctx, abortController } = makeStartAccountCtx({ + enabled: true, + token: "t", + incomingUrl: "https://nas/incoming", + dmPolicy: "open", + allowedUserIds: [], + }); + + const result = plugin.gateway.startAccount(ctx); + await expectPendingStartAccountPromise(result, abortController); + expect(ctx.log.warn).toHaveBeenCalledWith( + expect.stringContaining("dmPolicy=open but empty allowedUserIds"), + ); + expect(registerMock).not.toHaveBeenCalled(); + }); + it("startAccount refuses named accounts without explicit webhookPath in multi-account setups", async () => { const registerMock = registerSynologyWebhookRouteMock; const plugin = createSynologyChatPlugin(); @@ -553,6 +580,7 @@ describe("createSynologyChatPlugin", () => { const { ctx, abortController } = makeNamedStartAccountCtx({ webhookPath: "/webhook/synology-shared", dmPolicy: "open", + allowedUserIds: ["*"], }); const result = plugin.gateway.startAccount(ctx); diff --git a/extensions/synology-chat/src/channel.ts b/extensions/synology-chat/src/channel.ts index eec6fba9411..02264bc8259 100644 --- a/extensions/synology-chat/src/channel.ts +++ b/extensions/synology-chat/src/channel.ts @@ -116,11 +116,16 @@ const collectSynologyChatSecurityWarnings = "- Synology Chat: dangerouslyAllowInheritedWebhookPath=true opts a named account into a shared inherited webhook path. Prefer an explicit per-account webhookPath.", (account) => account.dmPolicy === "open" && + account.allowedUserIds.length === 0 && + '- Synology Chat: dmPolicy="open" with empty allowedUserIds blocks all senders. Add allowedUserIds=["*"] for public DMs or set explicit user IDs.', + (account) => + account.dmPolicy === "open" && + account.allowedUserIds.includes("*") && '- Synology Chat: dmPolicy="open" allows any user to message the bot. Consider "allowlist" for production use.', (account) => account.dmPolicy === "allowlist" && account.allowedUserIds.length === 0 && - '- Synology Chat: dmPolicy="allowlist" with empty allowedUserIds blocks all senders. Add users or set dmPolicy="open".', + '- Synology Chat: dmPolicy="allowlist" with empty allowedUserIds blocks all senders. Add users or set dmPolicy="open" with allowedUserIds=["*"].', ); type SynologyChatOutboundResult = { diff --git a/extensions/synology-chat/src/core.test.ts b/extensions/synology-chat/src/core.test.ts index 3ffef2bac73..6eeec16e42e 100644 --- a/extensions/synology-chat/src/core.test.ts +++ b/extensions/synology-chat/src/core.test.ts @@ -317,7 +317,12 @@ describe("synology-chat security helpers", () => { expect(checkUserAllowed("user1", ["user1", "user2"])).toBe(true); expect(checkUserAllowed("user3", ["user1", "user2"])).toBe(false); - expect(authorizeUserForDm("user1", "open", [])).toEqual({ allowed: true }); + expect(authorizeUserForDm("user1", "open", [])).toEqual({ + allowed: false, + reason: "not-allowlisted", + }); + expect(authorizeUserForDm("user1", "open", ["*"])).toEqual({ allowed: true }); + expect(authorizeUserForDm("user1", "open", ["user1"])).toEqual({ allowed: true }); expect(authorizeUserForDm("user1", "disabled", ["user1"])).toEqual({ allowed: false, reason: "disabled", diff --git a/extensions/synology-chat/src/gateway-runtime.ts b/extensions/synology-chat/src/gateway-runtime.ts index baec9c39f59..c900598b6b9 100644 --- a/extensions/synology-chat/src/gateway-runtime.ts +++ b/extensions/synology-chat/src/gateway-runtime.ts @@ -16,6 +16,7 @@ type SynologyGatewayStartupIssueCode = | "disabled" | "missing-credentials" | "empty-allowlist" + | "empty-open-allowlist" | "inherited-shared-webhook-path" | "duplicate-webhook-path"; type SynologyGatewayStartupIssue = { @@ -97,6 +98,14 @@ export function collectSynologyGatewayStartupIssues(params: { ), ); } + if (account.dmPolicy === "open" && account.allowedUserIds.length === 0) { + issues.push( + buildStartupIssue( + "empty-open-allowlist", + `account ${accountId} has dmPolicy=open but empty allowedUserIds; add allowedUserIds=["*"] for public DMs or set explicit user IDs`, + ), + ); + } const accountIds = listAccountIds(cfg); const isMultiAccount = accountIds.length > 1; diff --git a/extensions/synology-chat/src/security.ts b/extensions/synology-chat/src/security.ts index 61a4429486b..f30eca925a0 100644 --- a/extensions/synology-chat/src/security.ts +++ b/extensions/synology-chat/src/security.ts @@ -31,6 +31,9 @@ export function checkUserAllowed(userId: string, allowedUserIds: string[]): bool if (allowedUserIds.length === 0) { return false; } + if (allowedUserIds.includes("*")) { + return true; + } return allowedUserIds.includes(userId); } @@ -47,7 +50,9 @@ export function authorizeUserForDm( return { allowed: false, reason: "disabled" }; } if (dmPolicy === "open") { - return { allowed: true }; + return checkUserAllowed(userId, allowedUserIds) + ? { allowed: true } + : { allowed: false, reason: "not-allowlisted" }; } if (allowedUserIds.length === 0) { return { allowed: false, reason: "allowlist-empty" }; diff --git a/extensions/synology-chat/src/setup-surface.ts b/extensions/synology-chat/src/setup-surface.ts index a933118dccb..7364389668e 100644 --- a/extensions/synology-chat/src/setup-surface.ts +++ b/extensions/synology-chat/src/setup-surface.ts @@ -329,7 +329,7 @@ export const synologyChatSetupWizard: ChannelSetupWizard = { title: "Synology Chat access control", lines: [ `Default outgoing webhook path: ${DEFAULT_WEBHOOK_PATH}`, - 'Set allowed user IDs, or manually switch `channels.synology-chat.dmPolicy` to `"open"` for public DMs.', + 'Set allowed user IDs, or manually switch `channels.synology-chat.dmPolicy` to `"open"` with `allowedUserIds: ["*"]` for public DMs.', 'With `dmPolicy="allowlist"`, an empty allowedUserIds list blocks the route from starting.', `Docs: ${formatDocsLink("/channels/synology-chat", "channels/synology-chat")}`, ], diff --git a/extensions/synology-chat/src/webhook-handler.test.ts b/extensions/synology-chat/src/webhook-handler.test.ts index ff89a14bf9f..a69a801f705 100644 --- a/extensions/synology-chat/src/webhook-handler.test.ts +++ b/extensions/synology-chat/src/webhook-handler.test.ts @@ -30,7 +30,7 @@ function makeAccount( dangerouslyAllowNameMatching: false, dangerouslyAllowInheritedWebhookPath: false, dmPolicy: "open", - allowedUserIds: [], + allowedUserIds: ["*"], rateLimitPerMinute: 30, botName: "TestBot", allowInsecureSsl: true, diff --git a/extensions/synology-chat/src/webhook-handler.ts b/extensions/synology-chat/src/webhook-handler.ts index eaf72b5cb67..4622750f972 100644 --- a/extensions/synology-chat/src/webhook-handler.ts +++ b/extensions/synology-chat/src/webhook-handler.ts @@ -442,7 +442,8 @@ function authorizeSynologyWebhook(params: { return { ok: false, statusCode: 403, - error: "Allowlist is empty. Configure allowedUserIds or use dmPolicy=open.", + error: + 'Allowlist is empty. Configure allowedUserIds or use dmPolicy=open with allowedUserIds=["*"].', }; } params.log?.warn(`Unauthorized user: ${params.payload.user_id}`); diff --git a/extensions/telegram/src/account-config.ts b/extensions/telegram/src/account-config.ts index 4b3622b6734..056d1564ad0 100644 --- a/extensions/telegram/src/account-config.ts +++ b/extensions/telegram/src/account-config.ts @@ -5,6 +5,45 @@ import { } from "openclaw/plugin-sdk/account-core"; import type { TelegramAccountConfig } from "openclaw/plugin-sdk/config-types"; +function normalizeAllowFromEntry(value: string | number): string { + return String(value).trim(); +} + +function hasWildcardAllowFrom(value: unknown): boolean { + return ( + Array.isArray(value) && + value.some((entry) => normalizeAllowFromEntry(entry as string | number) === "*") + ); +} + +function hasRestrictiveAllowFrom(value: unknown): value is Array { + return ( + Array.isArray(value) && + value.some((entry) => { + const normalized = normalizeAllowFromEntry(entry as string | number); + return normalized.length > 0 && normalized !== "*"; + }) + ); +} + +function dropWildcardAllowFrom(value: Array): Array { + return value.filter((entry) => normalizeAllowFromEntry(entry) !== "*"); +} + +function resolveMergedAllowFrom(params: { + baseAllowFrom?: Array; + accountAllowFrom?: Array; +}): Array | undefined { + const { baseAllowFrom, accountAllowFrom } = params; + if (hasRestrictiveAllowFrom(baseAllowFrom) && hasWildcardAllowFrom(accountAllowFrom)) { + const accountRestrictiveEntries = Array.isArray(accountAllowFrom) + ? dropWildcardAllowFrom(accountAllowFrom) + : []; + return accountRestrictiveEntries.length > 0 ? accountRestrictiveEntries : baseAllowFrom; + } + return accountAllowFrom ?? baseAllowFrom; +} + export function resolveTelegramAccountConfig( cfg: OpenClawConfig, accountId: string, @@ -32,6 +71,10 @@ export function mergeTelegramAccountConfig( const configuredAccountIds = Object.keys(cfg.channels?.telegram?.accounts ?? {}); const isMultiAccount = configuredAccountIds.length > 1; const groups = account.groups ?? (isMultiAccount ? undefined : channelGroups); + const allowFrom = resolveMergedAllowFrom({ + baseAllowFrom: base.allowFrom, + accountAllowFrom: account.allowFrom, + }); - return { ...base, ...account, groups }; + return { ...base, ...account, allowFrom, groups }; } diff --git a/extensions/telegram/src/accounts.test.ts b/extensions/telegram/src/accounts.test.ts index 40a27669be1..a58b1c5087c 100644 --- a/extensions/telegram/src/accounts.test.ts +++ b/extensions/telegram/src/accounts.test.ts @@ -378,6 +378,54 @@ describe("mergeTelegramAccountConfig", () => { groupPolicy: "allowlist", }); }); + + it("drops account wildcard DM access when top-level allowFrom is restrictive", () => { + const cfg: OpenClawConfig = { + channels: { + telegram: { + enabled: true, + dmPolicy: "allowlist", + allowFrom: ["123"], + accounts: { + alerts: { + enabled: true, + botToken: "bot-token", + dmPolicy: "open", + allowFrom: ["*"], + }, + }, + }, + }, + }; + + expect(mergeTelegramAccountConfig(cfg, "alerts")).toMatchObject({ + botToken: "bot-token", + dmPolicy: "open", + allowFrom: ["123"], + }); + }); + + it("keeps explicit account allowlist entries while dropping a conflicting wildcard", () => { + const cfg: OpenClawConfig = { + channels: { + telegram: { + enabled: true, + allowFrom: ["123"], + accounts: { + alerts: { + botToken: "bot-token", + dmPolicy: "open", + allowFrom: ["456", "*"], + }, + }, + }, + }, + }; + + expect(mergeTelegramAccountConfig(cfg, "alerts")).toMatchObject({ + allowFrom: ["456"], + }); + }); }); describe("resolveTelegramPollActionGateState", () => { diff --git a/extensions/telegram/src/bot-handlers.runtime.ts b/extensions/telegram/src/bot-handlers.runtime.ts index 383728497ca..644923386aa 100644 --- a/extensions/telegram/src/bot-handlers.runtime.ts +++ b/extensions/telegram/src/bot-handlers.runtime.ts @@ -767,18 +767,20 @@ export const registerTelegramHandlers = ({ ); return { allowed: false, reason: "direct-disabled" }; } - if (dmPolicy !== "open") { - // For DMs, prefer per-DM/topic allowFrom (groupAllowOverride) over account-level allowFrom - const dmAllowFrom = groupAllowOverride ?? allowFrom; - const effectiveDmAllow = normalizeDmAllowFromWithStore({ - allowFrom: dmAllowFrom, - storeAllowFrom, - dmPolicy, - }); - if (!isAllowlistAuthorized(effectiveDmAllow, senderId, senderUsername)) { - logVerbose(`Blocked telegram direct sender ${senderId || "unknown"} (${deniedDmReason})`); - return { allowed: false, reason: "direct-unauthorized" }; - } + // For DMs, prefer per-DM/topic allowFrom (groupAllowOverride) over account-level allowFrom. + const dmAllowFrom = groupAllowOverride ?? allowFrom; + const effectiveDmAllow = normalizeDmAllowFromWithStore({ + allowFrom: dmAllowFrom, + storeAllowFrom, + dmPolicy, + }); + const hasPublicDmAccess = dmPolicy === "open" && effectiveDmAllow.hasWildcard; + if ( + !hasPublicDmAccess && + !isAllowlistAuthorized(effectiveDmAllow, senderId, senderUsername) + ) { + logVerbose(`Blocked telegram direct sender ${senderId || "unknown"} (${deniedDmReason})`); + return { allowed: false, reason: "direct-unauthorized" }; } } if (isGroup && enforceGroupAllowlistAuthorization) { diff --git a/extensions/telegram/src/bot.create-telegram-bot.test.ts b/extensions/telegram/src/bot.create-telegram-bot.test.ts index 35d84f18a90..d62f661a302 100644 --- a/extensions/telegram/src/bot.create-telegram-bot.test.ts +++ b/extensions/telegram/src/bot.create-telegram-bot.test.ts @@ -1672,10 +1672,12 @@ describe("createTelegramBot", () => { work: { botToken: "tok-work", dmPolicy: "open", + allowFrom: ["*"], }, opie: { botToken: "tok-opie", dmPolicy: "open", + allowFrom: ["*"], }, }, }, @@ -1781,10 +1783,12 @@ describe("createTelegramBot", () => { work: { botToken: "tok-work", dmPolicy: "open", + allowFrom: ["*"], }, opie: { botToken: "tok-opie", dmPolicy: "open", + allowFrom: ["*"], }, }, }, @@ -3129,7 +3133,7 @@ describe("createTelegramBot", () => { it("retries reaction updates after a bubbled enqueue failure", async () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "all" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, }, }); diff --git a/extensions/telegram/src/bot.test.ts b/extensions/telegram/src/bot.test.ts index 8fcffcf5549..6ccda9f6a59 100644 --- a/extensions/telegram/src/bot.test.ts +++ b/extensions/telegram/src/bot.test.ts @@ -2429,7 +2429,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "all" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, }, }); @@ -2520,6 +2520,34 @@ describe("createTelegramBot", () => { }, expectedEnqueueCalls: 1, }, + { + name: "blocks reaction in open mode when wildcard access was constrained", + updateId: 515, + channelConfig: { dmPolicy: "open", allowFrom: ["12345"], reactionNotifications: "all" }, + reaction: { + chat: { id: 1234, type: "private" }, + message_id: 42, + user: { id: 9, first_name: "Ada" }, + date: 1736380800, + old_reaction: [], + new_reaction: [{ type: "emoji", emoji: THUMBS_UP_EMOJI }], + }, + expectedEnqueueCalls: 0, + }, + { + name: "allows reaction in open mode with explicit wildcard access", + updateId: 516, + channelConfig: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, + reaction: { + chat: { id: 1234, type: "private" }, + message_id: 42, + user: { id: 9, first_name: "Ada" }, + date: 1736380800, + old_reaction: [], + new_reaction: [{ type: "emoji", emoji: THUMBS_UP_EMOJI }], + }, + expectedEnqueueCalls: 1, + }, { name: "blocks reaction in group allowlist mode for unauthorized sender", updateId: 513, @@ -2600,7 +2628,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open" }, + telegram: { dmPolicy: "open", allowFrom: ["*"] }, }, }); @@ -2631,7 +2659,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "all" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, }, }); @@ -2666,7 +2694,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "own" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "own" }, }, }); @@ -2697,7 +2725,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "own" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "own" }, }, }); @@ -2728,7 +2756,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "all" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, }, }); @@ -2758,7 +2786,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "all" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, }, }); @@ -2788,7 +2816,7 @@ describe("createTelegramBot", () => { loadConfig.mockReturnValue({ channels: { - telegram: { dmPolicy: "open", reactionNotifications: "all" }, + telegram: { dmPolicy: "open", allowFrom: ["*"], reactionNotifications: "all" }, }, }); diff --git a/extensions/telegram/src/dm-access.test.ts b/extensions/telegram/src/dm-access.test.ts index caa53c84966..e5adf0147d1 100644 --- a/extensions/telegram/src/dm-access.test.ts +++ b/extensions/telegram/src/dm-access.test.ts @@ -80,8 +80,31 @@ describe("enforceTelegramDmAccess", () => { vi.clearAllMocks(); }); - it("allows DMs when policy is open", async () => { - const { allowed, bot } = await enforceDefaultDmAccess({ dmPolicy: "open" }); + it("allows DMs when policy is open with wildcard allowFrom", async () => { + const { allowed, bot } = await enforceDefaultDmAccess({ + dmPolicy: "open", + allow: ["*"], + }); + + expect(allowed).toBe(true); + expect(bot.api.sendMessage).not.toHaveBeenCalled(); + }); + + it("blocks non-allowlisted DMs when open policy has no wildcard", async () => { + const { allowed, bot } = await enforceDefaultDmAccess({ + dmPolicy: "open", + allow: ["99999"], + }); + + expect(allowed).toBe(false); + expect(bot.api.sendMessage).not.toHaveBeenCalled(); + }); + + it("allows allowlisted DMs when open policy was constrained by a restrictive allowFrom", async () => { + const { allowed, bot } = await enforceDefaultDmAccess({ + dmPolicy: "open", + allow: ["12345"], + }); expect(allowed).toBe(true); expect(bot.api.sendMessage).not.toHaveBeenCalled(); diff --git a/extensions/telegram/src/dm-access.ts b/extensions/telegram/src/dm-access.ts index 5bbdac1863a..95095716208 100644 --- a/extensions/telegram/src/dm-access.ts +++ b/extensions/telegram/src/dm-access.ts @@ -60,9 +60,6 @@ export async function enforceTelegramDmAccess(params: { if (dmPolicy === "disabled") { return false; } - if (dmPolicy === "open") { - return true; - } const sender = resolveTelegramSenderIdentity(msg, chatId); const allowMatch = resolveSenderAllowMatch({ @@ -75,6 +72,15 @@ export async function enforceTelegramDmAccess(params: { }`; const allowed = effectiveDmAllow.hasWildcard || (effectiveDmAllow.hasEntries && allowMatch.allowed); + if (dmPolicy === "open") { + if (allowed) { + return true; + } + logVerbose( + `Blocked unauthorized telegram sender ${sender.candidateId} (dmPolicy=open, ${allowMatchMeta})`, + ); + return false; + } if (allowed) { return true; } diff --git a/extensions/zalouser/src/monitor.ts b/extensions/zalouser/src/monitor.ts index b565a6e1244..39956ddcea8 100644 --- a/extensions/zalouser/src/monitor.ts +++ b/extensions/zalouser/src/monitor.ts @@ -365,12 +365,8 @@ async function processMessage( groupPolicy, groupAllowFrom: configGroupAllowFrom, }); - const shouldComputeCommandAuth = core.channel.commands.shouldComputeCommandAuthorized( - commandBody, - config, - ); const storeAllowFrom = - !isGroup && dmPolicy !== "allowlist" && (dmPolicy !== "open" || shouldComputeCommandAuth) + !isGroup && dmPolicy !== "allowlist" && dmPolicy !== "open" ? await pairing.readAllowFromStore().catch(() => []) : []; const accessDecision = resolveDmGroupAccessWithLists({ diff --git a/src/channels/allow-from.ts b/src/channels/allow-from.ts index a7d44b44ab2..ca9c5cddfc0 100644 --- a/src/channels/allow-from.ts +++ b/src/channels/allow-from.ts @@ -5,7 +5,10 @@ export function mergeDmAllowFromSources(params: { storeAllowFrom?: Array; dmPolicy?: string; }): string[] { - const storeEntries = params.dmPolicy === "allowlist" ? [] : (params.storeAllowFrom ?? []); + const storeEntries = + params.dmPolicy === "allowlist" || params.dmPolicy === "open" + ? [] + : (params.storeAllowFrom ?? []); return normalizeStringEntries([...(params.allowFrom ?? []), ...storeEntries]); } diff --git a/src/plugin-sdk/channel-policy.ts b/src/plugin-sdk/channel-policy.ts index b056249ad76..533e7fc2fd2 100644 --- a/src/plugin-sdk/channel-policy.ts +++ b/src/plugin-sdk/channel-policy.ts @@ -49,6 +49,7 @@ export { resolveDmGroupAccessWithCommandGate, resolveDmGroupAccessWithLists, resolveEffectiveAllowFromLists, + resolveOpenDmAllowlistAccess, } from "../security/dm-policy-shared.js"; export { evaluateGroupRouteAccessForPolicy, diff --git a/src/plugin-sdk/command-auth.test.ts b/src/plugin-sdk/command-auth.test.ts index 0fe5834a731..c1fce2b6d54 100644 --- a/src/plugin-sdk/command-auth.test.ts +++ b/src/plugin-sdk/command-auth.test.ts @@ -91,4 +91,25 @@ describe("plugin-sdk/command-auth", () => { expect(result.senderAllowedForCommands).toBe(true); expect(result.commandAuthorized).toBeUndefined(); }); + + it("does not treat open DM policy as an allowlist bypass", async () => { + const result = await resolveSenderCommandAuthorization({ + cfg: baseCfg, + rawBody: "hello", + isGroup: false, + dmPolicy: "open", + configuredAllowFrom: [], + configuredGroupAllowFrom: [], + senderId: "paired-user", + isSenderAllowed: (senderId, allowFrom) => allowFrom.includes(senderId), + readAllowFromStore: async () => ["paired-user"], + shouldComputeCommandAuthorized: (rawBody) => rawBody.startsWith("/"), + resolveCommandAuthorizedFromAuthorizers: ({ useAccessGroups, authorizers }) => + useAccessGroups && authorizers.some((entry) => entry.configured && entry.allowed), + }); + + expect(result.effectiveAllowFrom).toEqual([]); + expect(result.senderAllowedForCommands).toBe(false); + expect(result.commandAuthorized).toBeUndefined(); + }); }); diff --git a/src/plugin-sdk/command-auth.ts b/src/plugin-sdk/command-auth.ts index 192b71b7b83..c2d45594046 100644 --- a/src/plugin-sdk/command-auth.ts +++ b/src/plugin-sdk/command-auth.ts @@ -132,7 +132,7 @@ export function resolveDirectDmAuthorizationOutcome(params: { if (params.dmPolicy === "disabled") { return "disabled"; } - if (params.dmPolicy !== "open" && !params.senderAllowedForCommands) { + if (!params.senderAllowedForCommands) { return "unauthorized"; } return "allowed"; @@ -161,9 +161,7 @@ export async function resolveSenderCommandAuthorization( }> { const shouldComputeAuth = params.shouldComputeCommandAuthorized(params.rawBody, params.cfg); const storeAllowFrom = - !params.isGroup && - params.dmPolicy !== "allowlist" && - (params.dmPolicy !== "open" || shouldComputeAuth) + !params.isGroup && params.dmPolicy !== "allowlist" && params.dmPolicy !== "open" ? await params.readAllowFromStore().catch(() => []) : []; const access = resolveDmGroupAccessWithLists({ diff --git a/src/plugin-sdk/direct-dm-access.ts b/src/plugin-sdk/direct-dm-access.ts index 4ec6f656354..63ce94bde0a 100644 --- a/src/plugin-sdk/direct-dm-access.ts +++ b/src/plugin-sdk/direct-dm-access.ts @@ -70,18 +70,16 @@ export async function resolveInboundDirectDmAccessWithRuntime(params: { access.effectiveAllowFrom, ); const commandAuthorized = shouldComputeAuth - ? dmPolicy === "open" - ? true - : params.runtime.resolveCommandAuthorizedFromAuthorizers({ - useAccessGroups: params.cfg.commands?.useAccessGroups !== false, - authorizers: [ - { - configured: access.effectiveAllowFrom.length > 0, - allowed: senderAllowedForCommands, - }, - ], - modeWhenAccessGroupsOff: params.modeWhenAccessGroupsOff, - }) + ? params.runtime.resolveCommandAuthorizedFromAuthorizers({ + useAccessGroups: params.cfg.commands?.useAccessGroups !== false, + authorizers: [ + { + configured: access.effectiveAllowFrom.length > 0, + allowed: senderAllowedForCommands, + }, + ], + modeWhenAccessGroupsOff: params.modeWhenAccessGroupsOff, + }) : undefined; return { diff --git a/src/plugin-sdk/direct-dm.test.ts b/src/plugin-sdk/direct-dm.test.ts index 8a763180616..21660a265ce 100644 --- a/src/plugin-sdk/direct-dm.test.ts +++ b/src/plugin-sdk/direct-dm.test.ts @@ -70,6 +70,29 @@ describe("plugin-sdk/direct-dm", () => { expect(result.commandAuthorized).toBe(true); }); + it("blocks open DMs unless the effective allowlist matches", async () => { + const result = await resolveInboundDirectDmAccessWithRuntime({ + cfg: baseCfg, + channel: "nostr", + accountId: "default", + dmPolicy: "open", + allowFrom: [], + senderId: "random-user", + rawBody: "hello", + isSenderAllowed: (senderId, allowFrom) => allowFrom.includes(senderId), + readStoreAllowFrom: async () => ["random-user"], + runtime: { + shouldComputeCommandAuthorized: () => false, + resolveCommandAuthorizedFromAuthorizers: () => true, + }, + }); + + expect(result.access.decision).toBe("block"); + expect(result.access.reason).toBe("dmPolicy=open (not allowlisted)"); + expect(result.access.effectiveAllowFrom).toEqual([]); + expect(result.commandAuthorized).toBeUndefined(); + }); + it("creates a pre-crypto authorizer that issues pairing and blocks unknown senders", async () => { const issuePairingChallenge = vi.fn(async () => {}); const onBlocked = vi.fn(); diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts index a7888518e8b..df3bb252bba 100644 --- a/src/security/dm-policy-shared.test.ts +++ b/src/security/dm-policy-shared.test.ts @@ -110,6 +110,14 @@ describe("security/dm-policy-shared", () => { dmPolicy: "allowlist" as const, }, }, + { + name: "dmPolicy is open", + params: { + provider: "demo-channel-open", + accountId: "default", + dmPolicy: "open" as const, + }, + }, { name: "shouldRead=false", params: { @@ -203,6 +211,17 @@ describe("security/dm-policy-shared", () => { expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc"]); }); + it("excludes pairing-store entries when dmPolicy is open", () => { + const lists = resolveEffectiveAllowFromLists({ + allowFrom: ["owner"], + groupAllowFrom: ["group:abc"], + storeAllowFrom: ["paired-user"], + dmPolicy: "open", + }); + expect(lists.effectiveAllowFrom).toEqual(["owner"]); + expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc"]); + }); + it("keeps group allowlist explicit when dmPolicy is pairing", () => { const lists = resolveEffectiveAllowFromLists({ allowFrom: ["+1111"], @@ -278,11 +297,34 @@ describe("security/dm-policy-shared", () => { isSenderAllowed: () => false, command: controlCommand, }); - expect(resolved.decision).toBe("allow"); + expect(resolved.decision).toBe("block"); + expect(resolved.reasonCode).toBe(DM_GROUP_ACCESS_REASON.DM_POLICY_NOT_ALLOWLISTED); + expect(resolved.reason).toBe("dmPolicy=open (not allowlisted)"); expect(resolved.commandAuthorized).toBe(false); expect(resolved.shouldBlockControlCommand).toBe(false); }); + it("allows open-mode DMs only for wildcard or matching allowlist entries", () => { + const publicAccess = resolveDmGroupAccessWithLists({ + isGroup: false, + dmPolicy: "open", + allowFrom: ["*"], + isSenderAllowed: () => true, + }); + expect(publicAccess.decision).toBe("allow"); + expect(publicAccess.reasonCode).toBe(DM_GROUP_ACCESS_REASON.DM_POLICY_OPEN); + + const constrainedAccess = resolveDmGroupAccessWithLists({ + isGroup: false, + dmPolicy: "open", + allowFrom: ["owner"], + isSenderAllowed: (allowFrom) => allowFrom.includes("owner"), + }); + expect(constrainedAccess.decision).toBe("allow"); + expect(constrainedAccess.reasonCode).toBe(DM_GROUP_ACCESS_REASON.DM_POLICY_ALLOWLISTED); + expect(constrainedAccess.reason).toBe("dmPolicy=open (allowlisted)"); + }); + it("keeps allowlist mode strict in shared resolver (no pairing-store fallback)", () => { const resolved = resolveDmGroupAccessWithLists({ isGroup: false, @@ -371,8 +413,16 @@ describe("security/dm-policy-shared", () => { channels.flatMap((channel) => [ createParityCase({ - name: "dmPolicy=open", + name: "dmPolicy=open without wildcard", dmPolicy: "open", + expectedDecision: "block", + expectedReactionAllowed: false, + }), + createParityCase({ + name: "dmPolicy=open with wildcard", + dmPolicy: "open", + allowFrom: ["*"], + isSenderAllowed: (allowFrom: string[]) => allowFrom.includes("*"), expectedDecision: "allow", expectedReactionAllowed: true, }), diff --git a/src/security/dm-policy-shared.ts b/src/security/dm-policy-shared.ts index 430607b72a4..9fc9f40e5e7 100644 --- a/src/security/dm-policy-shared.ts +++ b/src/security/dm-policy-shared.ts @@ -74,6 +74,36 @@ export const DM_GROUP_ACCESS_REASON = { export type DmGroupAccessReasonCode = (typeof DM_GROUP_ACCESS_REASON)[keyof typeof DM_GROUP_ACCESS_REASON]; +export function resolveOpenDmAllowlistAccess(params: { + effectiveAllowFrom: Array; + isSenderAllowed: (allowFrom: string[]) => boolean; +}): { + decision: Extract; + reasonCode: DmGroupAccessReasonCode; + reason: string; +} { + const effectiveAllowFrom = normalizeStringEntries(params.effectiveAllowFrom); + if (effectiveAllowFrom.includes("*")) { + return { + decision: "allow", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_OPEN, + reason: "dmPolicy=open", + }; + } + if (params.isSenderAllowed(effectiveAllowFrom)) { + return { + decision: "allow", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_ALLOWLISTED, + reason: "dmPolicy=open (allowlisted)", + }; + } + return { + decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_NOT_ALLOWLISTED, + reason: "dmPolicy=open (not allowlisted)", + }; +} + type DmGroupAccessInputParams = { isGroup: boolean; dmPolicy?: string | null; @@ -92,7 +122,11 @@ export async function readStoreAllowFromForDmPolicy(params: { shouldRead?: boolean | null; readStore?: (provider: ChannelId, accountId: string) => Promise; }): Promise { - if (params.shouldRead === false || params.dmPolicy === "allowlist") { + if ( + params.shouldRead === false || + params.dmPolicy === "allowlist" || + params.dmPolicy === "open" + ) { return []; } const readStore = @@ -168,11 +202,10 @@ export function resolveDmGroupAccessDecision(params: { }; } if (dmPolicy === "open") { - return { - decision: "allow", - reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_OPEN, - reason: "dmPolicy=open", - }; + return resolveOpenDmAllowlistAccess({ + effectiveAllowFrom, + isSenderAllowed: params.isSenderAllowed, + }); } if (params.isSenderAllowed(effectiveAllowFrom)) { return { @@ -234,6 +267,7 @@ export function resolveDmGroupAccessWithCommandGate( }, ): { decision: DmGroupAccessDecision; + reasonCode: DmGroupAccessReasonCode; reason: string; effectiveAllowFrom: string[]; effectiveGroupAllowFrom: string[];