mirror of
https://fastgit.cc/github.com/openclaw/openclaw
synced 2026-04-30 14:02:56 +08:00
fix: align open DM allowlist policy (#74112)
* fix: harden telegram open dm allowlist merging * fix: align open dm allowlist policy
This commit is contained in:
committed by
GitHub
parent
fda8cc2a9d
commit
bd1d1f0f2b
@@ -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 `<system-reminder>` and `<previous_response>` 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.
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:**
|
||||
|
||||
@@ -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`).
|
||||
|
||||
@@ -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`
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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<string[]>;
|
||||
}): Promise<DiscordDmCommandAccess> {
|
||||
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,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -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: ["*"],
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -35,6 +35,7 @@ function buildConfig(overrides?: Partial<ClawdbotConfig>): ClawdbotConfig {
|
||||
feishu: {
|
||||
enabled: true,
|
||||
dmPolicy: "open",
|
||||
allowFrom: ["*"],
|
||||
},
|
||||
},
|
||||
...overrides,
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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"],
|
||||
});
|
||||
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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<typeof handleLineWebhookEvents>[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,
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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"],
|
||||
|
||||
@@ -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<string | number>;
|
||||
storeAllowFrom: Array<string | number>;
|
||||
dmPolicy?: "open" | "pairing" | "allowlist" | "disabled";
|
||||
groupAllowFrom: Array<string | number>;
|
||||
roomUsers: Array<string | number>;
|
||||
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;
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -1614,7 +1614,7 @@ describe("matrix monitor handler pairing account scope", () => {
|
||||
channels: {
|
||||
matrix: {
|
||||
threadReplies: "always",
|
||||
dm: { threadReplies: "off" },
|
||||
dm: { allowFrom: ["*"], threadReplies: "off" },
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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: [],
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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)",
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -31,7 +31,7 @@ export function createInboundSlackTestContext(params: {
|
||||
mainKey: "main",
|
||||
dmEnabled: true,
|
||||
dmPolicy: "open",
|
||||
allowFrom: [],
|
||||
allowFrom: ["*"],
|
||||
allowNameMatching: false,
|
||||
groupDmEnabled: true,
|
||||
groupDmChannels: [],
|
||||
|
||||
@@ -148,6 +148,7 @@ describe("slack prepareSlackMessage inbound contract", () => {
|
||||
followUpTs: string;
|
||||
currentTs: string;
|
||||
channelsConfig?: Parameters<typeof createInboundSlackCtx>[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(
|
||||
|
||||
@@ -416,9 +416,7 @@ export async function prepareSlackMessage(params: {
|
||||
? normalizeAllowListLower(channelConfig?.users)
|
||||
: []
|
||||
: isDirectMessage
|
||||
? ctx.dmPolicy === "open"
|
||||
? []
|
||||
: allowFromLower
|
||||
? allowFromLower
|
||||
: [];
|
||||
const contextVisibilityMode = resolveChannelContextVisibilityMode({
|
||||
cfg: ctx.cfg,
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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: ["*"],
|
||||
},
|
||||
},
|
||||
},
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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" };
|
||||
|
||||
@@ -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")}`,
|
||||
],
|
||||
|
||||
@@ -30,7 +30,7 @@ function makeAccount(
|
||||
dangerouslyAllowNameMatching: false,
|
||||
dangerouslyAllowInheritedWebhookPath: false,
|
||||
dmPolicy: "open",
|
||||
allowedUserIds: [],
|
||||
allowedUserIds: ["*"],
|
||||
rateLimitPerMinute: 30,
|
||||
botName: "TestBot",
|
||||
allowInsecureSsl: true,
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
@@ -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<string | number> {
|
||||
return (
|
||||
Array.isArray(value) &&
|
||||
value.some((entry) => {
|
||||
const normalized = normalizeAllowFromEntry(entry as string | number);
|
||||
return normalized.length > 0 && normalized !== "*";
|
||||
})
|
||||
);
|
||||
}
|
||||
|
||||
function dropWildcardAllowFrom(value: Array<string | number>): Array<string | number> {
|
||||
return value.filter((entry) => normalizeAllowFromEntry(entry) !== "*");
|
||||
}
|
||||
|
||||
function resolveMergedAllowFrom(params: {
|
||||
baseAllowFrom?: Array<string | number>;
|
||||
accountAllowFrom?: Array<string | number>;
|
||||
}): Array<string | number> | 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 };
|
||||
}
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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" },
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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" },
|
||||
},
|
||||
});
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -5,7 +5,10 @@ export function mergeDmAllowFromSources(params: {
|
||||
storeAllowFrom?: Array<string | number>;
|
||||
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]);
|
||||
}
|
||||
|
||||
|
||||
@@ -49,6 +49,7 @@ export {
|
||||
resolveDmGroupAccessWithCommandGate,
|
||||
resolveDmGroupAccessWithLists,
|
||||
resolveEffectiveAllowFromLists,
|
||||
resolveOpenDmAllowlistAccess,
|
||||
} from "../security/dm-policy-shared.js";
|
||||
export {
|
||||
evaluateGroupRouteAccessForPolicy,
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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({
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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,
|
||||
}),
|
||||
|
||||
@@ -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<string | number>;
|
||||
isSenderAllowed: (allowFrom: string[]) => boolean;
|
||||
}): {
|
||||
decision: Extract<DmGroupAccessDecision, "allow" | "block">;
|
||||
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<string[]>;
|
||||
}): Promise<string[]> {
|
||||
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[];
|
||||
|
||||
Reference in New Issue
Block a user