mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 20:18:28 +00:00
fix: enforce strict allowlist across pairing stores (#23017)
This commit is contained in:
committed by
GitHub
parent
617e38cec0
commit
0bd9f0d4ac
@@ -10,6 +10,26 @@ describe("mergeAllowFromSources", () => {
|
||||
}),
|
||||
).toEqual(["line:user:abc", "123", "telegram:456"]);
|
||||
});
|
||||
|
||||
it("excludes pairing-store entries when dmPolicy is allowlist", () => {
|
||||
expect(
|
||||
mergeAllowFromSources({
|
||||
allowFrom: ["+1111"],
|
||||
storeAllowFrom: ["+2222", "+3333"],
|
||||
dmPolicy: "allowlist",
|
||||
}),
|
||||
).toEqual(["+1111"]);
|
||||
});
|
||||
|
||||
it("keeps pairing-store entries for non-allowlist policies", () => {
|
||||
expect(
|
||||
mergeAllowFromSources({
|
||||
allowFrom: ["+1111"],
|
||||
storeAllowFrom: ["+2222"],
|
||||
dmPolicy: "pairing",
|
||||
}),
|
||||
).toEqual(["+1111", "+2222"]);
|
||||
});
|
||||
});
|
||||
|
||||
describe("firstDefined", () => {
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
export function mergeAllowFromSources(params: {
|
||||
allowFrom?: Array<string | number>;
|
||||
storeAllowFrom?: string[];
|
||||
dmPolicy?: string;
|
||||
}): string[] {
|
||||
return [...(params.allowFrom ?? []), ...(params.storeAllowFrom ?? [])]
|
||||
const storeEntries = params.dmPolicy === "allowlist" ? [] : (params.storeAllowFrom ?? []);
|
||||
return [...(params.allowFrom ?? []), ...storeEntries]
|
||||
.map((value) => String(value).trim())
|
||||
.filter(Boolean);
|
||||
}
|
||||
|
||||
@@ -464,7 +464,8 @@ async function ensureDmComponentAuthorized(params: {
|
||||
return true;
|
||||
}
|
||||
|
||||
const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const storeAllowFrom =
|
||||
dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const effectiveAllowFrom = [...(ctx.allowFrom ?? []), ...storeAllowFrom];
|
||||
const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]);
|
||||
const allowMatch = allowList
|
||||
|
||||
@@ -178,7 +178,8 @@ export async function preflightDiscordMessage(
|
||||
return null;
|
||||
}
|
||||
if (dmPolicy !== "open") {
|
||||
const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const storeAllowFrom =
|
||||
dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const effectiveAllowFrom = [...(params.allowFrom ?? []), ...storeAllowFrom];
|
||||
const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]);
|
||||
const allowMatch = allowList
|
||||
|
||||
@@ -140,7 +140,7 @@ describe("agent components", () => {
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("allows DM interactions when pairing store allowlist matches", async () => {
|
||||
it("blocks DM interactions when only pairing store entries match in allowlist mode", async () => {
|
||||
readAllowFromStoreMock.mockResolvedValue(["123456789"]);
|
||||
const button = createAgentComponentButton({
|
||||
cfg: createCfg(),
|
||||
@@ -152,8 +152,9 @@ describe("agent components", () => {
|
||||
await button.run(interaction, { componentId: "hello" } as ComponentData);
|
||||
|
||||
expect(defer).toHaveBeenCalledWith({ ephemeral: true });
|
||||
expect(reply).toHaveBeenCalledWith({ content: "✓" });
|
||||
expect(enqueueSystemEventMock).toHaveBeenCalled();
|
||||
expect(reply).toHaveBeenCalledWith({ content: "You are not authorized to use this button." });
|
||||
expect(enqueueSystemEventMock).not.toHaveBeenCalled();
|
||||
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("matches tag-based allowlist entries for DM select menus", async () => {
|
||||
|
||||
@@ -1349,7 +1349,8 @@ async function dispatchDiscordCommandInteraction(params: {
|
||||
return;
|
||||
}
|
||||
if (dmPolicy !== "open") {
|
||||
const storeAllowFrom = await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const storeAllowFrom =
|
||||
dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("discord").catch(() => []);
|
||||
const effectiveAllowFrom = [
|
||||
...(discordConfig?.allowFrom ?? discordConfig?.dm?.allowFrom ?? []),
|
||||
...storeAllowFrom,
|
||||
|
||||
@@ -138,7 +138,8 @@ export function resolveIMessageInboundDecision(params: {
|
||||
}
|
||||
|
||||
const groupId = isGroup ? groupIdCandidate : undefined;
|
||||
const effectiveDmAllowFrom = Array.from(new Set([...params.allowFrom, ...params.storeAllowFrom]))
|
||||
const storeAllowFrom = params.dmPolicy === "allowlist" ? [] : params.storeAllowFrom;
|
||||
const effectiveDmAllowFrom = Array.from(new Set([...params.allowFrom, ...storeAllowFrom]))
|
||||
.map((v) => String(v).trim())
|
||||
.filter(Boolean);
|
||||
// Keep DM pairing-store authorization scoped to DMs; group access must come from explicit group allowlist config.
|
||||
|
||||
@@ -30,6 +30,7 @@ export const normalizeAllowFrom = (list?: Array<string | number>): NormalizedAll
|
||||
export const normalizeAllowFromWithStore = (params: {
|
||||
allowFrom?: Array<string | number>;
|
||||
storeAllowFrom?: string[];
|
||||
dmPolicy?: string;
|
||||
}): NormalizedAllowFrom => normalizeAllowFrom(mergeAllowFromSources(params));
|
||||
|
||||
export const isSenderAllowed = (params: {
|
||||
|
||||
@@ -109,11 +109,13 @@ async function shouldProcessLineEvent(
|
||||
const { cfg, account } = context;
|
||||
const { userId, groupId, roomId, isGroup } = getLineSourceInfo(event.source);
|
||||
const senderId = userId ?? "";
|
||||
const dmPolicy = account.config.dmPolicy ?? "pairing";
|
||||
|
||||
const storeAllowFrom = await readChannelAllowFromStore("line").catch(() => []);
|
||||
const effectiveDmAllow = normalizeAllowFromWithStore({
|
||||
allowFrom: account.config.allowFrom,
|
||||
storeAllowFrom,
|
||||
dmPolicy,
|
||||
});
|
||||
const groupConfig = resolveLineGroupConfig({ config: account.config, groupId, roomId });
|
||||
const groupAllowOverride = groupConfig?.allowFrom;
|
||||
@@ -128,8 +130,8 @@ async function shouldProcessLineEvent(
|
||||
const effectiveGroupAllow = normalizeAllowFromWithStore({
|
||||
allowFrom: groupAllowFrom,
|
||||
storeAllowFrom,
|
||||
dmPolicy,
|
||||
});
|
||||
const dmPolicy = account.config.dmPolicy ?? "pairing";
|
||||
const defaultGroupPolicy = cfg.channels?.defaults?.groupPolicy;
|
||||
const groupPolicy = account.config.groupPolicy ?? defaultGroupPolicy ?? "allowlist";
|
||||
|
||||
|
||||
@@ -26,7 +26,9 @@ export async function resolveSenderCommandAuthorization(
|
||||
}> {
|
||||
const shouldComputeAuth = params.shouldComputeCommandAuthorized(params.rawBody, params.cfg);
|
||||
const storeAllowFrom =
|
||||
!params.isGroup && (params.dmPolicy !== "open" || shouldComputeAuth)
|
||||
!params.isGroup &&
|
||||
params.dmPolicy !== "allowlist" &&
|
||||
(params.dmPolicy !== "open" || shouldComputeAuth)
|
||||
? await params.readAllowFromStore().catch(() => [])
|
||||
: [];
|
||||
const effectiveAllowFrom = [...params.configuredAllowFrom, ...storeAllowFrom];
|
||||
|
||||
@@ -53,6 +53,28 @@ describe("security/dm-policy-shared", () => {
|
||||
expect(lists.effectiveGroupAllowFrom).toEqual(["owner", "owner2"]);
|
||||
});
|
||||
|
||||
it("excludes storeAllowFrom when dmPolicy is allowlist", () => {
|
||||
const lists = resolveEffectiveAllowFromLists({
|
||||
allowFrom: ["+1111"],
|
||||
groupAllowFrom: ["group:abc"],
|
||||
storeAllowFrom: ["+2222", "+3333"],
|
||||
dmPolicy: "allowlist",
|
||||
});
|
||||
expect(lists.effectiveAllowFrom).toEqual(["+1111"]);
|
||||
expect(lists.effectiveGroupAllowFrom).toEqual(["group:abc"]);
|
||||
});
|
||||
|
||||
it("includes storeAllowFrom when dmPolicy is pairing", () => {
|
||||
const lists = resolveEffectiveAllowFromLists({
|
||||
allowFrom: ["+1111"],
|
||||
groupAllowFrom: [],
|
||||
storeAllowFrom: ["+2222"],
|
||||
dmPolicy: "pairing",
|
||||
});
|
||||
expect(lists.effectiveAllowFrom).toEqual(["+1111", "+2222"]);
|
||||
expect(lists.effectiveGroupAllowFrom).toEqual(["+1111", "+2222"]);
|
||||
});
|
||||
|
||||
const channels = [
|
||||
"bluebubbles",
|
||||
"imessage",
|
||||
|
||||
@@ -6,6 +6,7 @@ export function resolveEffectiveAllowFromLists(params: {
|
||||
allowFrom?: Array<string | number> | null;
|
||||
groupAllowFrom?: Array<string | number> | null;
|
||||
storeAllowFrom?: Array<string | number> | null;
|
||||
dmPolicy?: string | null;
|
||||
}): {
|
||||
effectiveAllowFrom: string[];
|
||||
effectiveGroupAllowFrom: string[];
|
||||
@@ -16,9 +17,12 @@ export function resolveEffectiveAllowFromLists(params: {
|
||||
const configGroupAllowFrom = normalizeStringEntries(
|
||||
Array.isArray(params.groupAllowFrom) ? params.groupAllowFrom : undefined,
|
||||
);
|
||||
const storeAllowFrom = normalizeStringEntries(
|
||||
Array.isArray(params.storeAllowFrom) ? params.storeAllowFrom : undefined,
|
||||
);
|
||||
const storeAllowFrom =
|
||||
params.dmPolicy === "allowlist"
|
||||
? []
|
||||
: normalizeStringEntries(
|
||||
Array.isArray(params.storeAllowFrom) ? params.storeAllowFrom : undefined,
|
||||
);
|
||||
const effectiveAllowFrom = normalizeStringEntries([...configAllowFrom, ...storeAllowFrom]);
|
||||
const groupBase = configGroupAllowFrom.length > 0 ? configGroupAllowFrom : configAllowFrom;
|
||||
const effectiveGroupAllowFrom = normalizeStringEntries([...groupBase, ...storeAllowFrom]);
|
||||
|
||||
@@ -441,7 +441,10 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) {
|
||||
const groupId = dataMessage.groupInfo?.groupId ?? undefined;
|
||||
const groupName = dataMessage.groupInfo?.groupName ?? undefined;
|
||||
const isGroup = Boolean(groupId);
|
||||
const storeAllowFrom = await readChannelAllowFromStore("signal").catch(() => []);
|
||||
const storeAllowFrom =
|
||||
deps.dmPolicy === "allowlist"
|
||||
? []
|
||||
: await readChannelAllowFromStore("signal").catch(() => []);
|
||||
const effectiveDmAllow = [...deps.allowFrom, ...storeAllowFrom];
|
||||
const effectiveGroupAllow = [...deps.groupAllowFrom, ...storeAllowFrom];
|
||||
const dmAllowed =
|
||||
|
||||
@@ -3,7 +3,8 @@ import { allowListMatches, normalizeAllowList, normalizeAllowListLower } from ".
|
||||
import type { SlackMonitorContext } from "./context.js";
|
||||
|
||||
export async function resolveSlackEffectiveAllowFrom(ctx: SlackMonitorContext) {
|
||||
const storeAllowFrom = await readChannelAllowFromStore("slack").catch(() => []);
|
||||
const storeAllowFrom =
|
||||
ctx.dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("slack").catch(() => []);
|
||||
const allowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]);
|
||||
const allowFromLower = normalizeAllowListLower(allowFrom);
|
||||
return { allowFrom, allowFromLower };
|
||||
|
||||
@@ -350,7 +350,10 @@ export async function registerSlackMonitorSlashCommands(params: {
|
||||
return;
|
||||
}
|
||||
|
||||
const storeAllowFrom = await readChannelAllowFromStore("slack").catch(() => []);
|
||||
const storeAllowFrom =
|
||||
ctx.dmPolicy === "allowlist"
|
||||
? []
|
||||
: await readChannelAllowFromStore("slack").catch(() => []);
|
||||
const effectiveAllowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]);
|
||||
const effectiveAllowFromLower = normalizeAllowListLower(effectiveAllowFrom);
|
||||
|
||||
|
||||
@@ -56,6 +56,7 @@ export const normalizeAllowFrom = (list?: Array<string | number>): NormalizedAll
|
||||
export const normalizeAllowFromWithStore = (params: {
|
||||
allowFrom?: Array<string | number>;
|
||||
storeAllowFrom?: string[];
|
||||
dmPolicy?: string;
|
||||
}): NormalizedAllowFrom => normalizeAllowFrom(mergeAllowFromSources(params));
|
||||
|
||||
export const isSenderAllowed = (params: {
|
||||
|
||||
@@ -794,6 +794,7 @@ export const registerTelegramHandlers = ({
|
||||
const groupAllowContext = await resolveTelegramGroupAllowFromContext({
|
||||
chatId,
|
||||
accountId,
|
||||
dmPolicy: telegramCfg.dmPolicy ?? "pairing",
|
||||
isForum,
|
||||
messageThreadId,
|
||||
groupAllowFrom,
|
||||
@@ -807,11 +808,12 @@ export const registerTelegramHandlers = ({
|
||||
effectiveGroupAllow,
|
||||
hasGroupAllowOverride,
|
||||
} = groupAllowContext;
|
||||
const dmPolicy = telegramCfg.dmPolicy ?? "pairing";
|
||||
const effectiveDmAllow = normalizeAllowFromWithStore({
|
||||
allowFrom: telegramCfg.allowFrom,
|
||||
storeAllowFrom,
|
||||
dmPolicy,
|
||||
});
|
||||
const dmPolicy = telegramCfg.dmPolicy ?? "pairing";
|
||||
const senderId = callback.from?.id ? String(callback.from.id) : "";
|
||||
const senderUsername = callback.from?.username ?? "";
|
||||
if (
|
||||
@@ -1089,6 +1091,7 @@ export const registerTelegramHandlers = ({
|
||||
const groupAllowContext = await resolveTelegramGroupAllowFromContext({
|
||||
chatId: event.chatId,
|
||||
accountId,
|
||||
dmPolicy: telegramCfg.dmPolicy ?? "pairing",
|
||||
isForum: event.isForum,
|
||||
messageThreadId: event.messageThreadId,
|
||||
groupAllowFrom,
|
||||
|
||||
@@ -197,11 +197,12 @@ export const buildTelegramMessageContext = async ({
|
||||
: null;
|
||||
const sessionKey = threadKeys?.sessionKey ?? baseSessionKey;
|
||||
const mentionRegexes = buildMentionRegexes(cfg, route.agentId);
|
||||
const effectiveDmAllow = normalizeAllowFromWithStore({ allowFrom, storeAllowFrom });
|
||||
const effectiveDmAllow = normalizeAllowFromWithStore({ allowFrom, storeAllowFrom, dmPolicy });
|
||||
const groupAllowOverride = firstDefined(topicConfig?.allowFrom, groupConfig?.allowFrom);
|
||||
const effectiveGroupAllow = normalizeAllowFromWithStore({
|
||||
allowFrom: groupAllowOverride ?? groupAllowFrom,
|
||||
storeAllowFrom,
|
||||
dmPolicy,
|
||||
});
|
||||
const hasGroupAllowOverride = typeof groupAllowOverride !== "undefined";
|
||||
const senderId = msg.from?.id ? String(msg.from.id) : "";
|
||||
|
||||
@@ -167,6 +167,7 @@ async function resolveTelegramCommandAuth(params: {
|
||||
const groupAllowContext = await resolveTelegramGroupAllowFromContext({
|
||||
chatId,
|
||||
accountId,
|
||||
dmPolicy: telegramCfg.dmPolicy ?? "pairing",
|
||||
isForum,
|
||||
messageThreadId,
|
||||
groupAllowFrom,
|
||||
@@ -251,6 +252,7 @@ async function resolveTelegramCommandAuth(params: {
|
||||
const dmAllow = normalizeAllowFromWithStore({
|
||||
allowFrom: allowFrom,
|
||||
storeAllowFrom,
|
||||
dmPolicy: telegramCfg.dmPolicy ?? "pairing",
|
||||
});
|
||||
const senderAllowed = isSenderAllowed({
|
||||
allow: dmAllow,
|
||||
|
||||
@@ -20,6 +20,7 @@ export type TelegramThreadSpec = {
|
||||
export async function resolveTelegramGroupAllowFromContext(params: {
|
||||
chatId: string | number;
|
||||
accountId?: string;
|
||||
dmPolicy?: string;
|
||||
isForum?: boolean;
|
||||
messageThreadId?: number | null;
|
||||
groupAllowFrom?: Array<string | number>;
|
||||
@@ -53,6 +54,7 @@ export async function resolveTelegramGroupAllowFromContext(params: {
|
||||
const effectiveGroupAllow = normalizeAllowFromWithStore({
|
||||
allowFrom: groupAllowOverride ?? params.groupAllowFrom,
|
||||
storeAllowFrom,
|
||||
dmPolicy: params.dmPolicy,
|
||||
});
|
||||
const hasGroupAllowOverride = typeof groupAllowOverride !== "undefined";
|
||||
return {
|
||||
|
||||
@@ -28,6 +28,7 @@ import { getAgentScopedMediaLocalRoots } from "../../../media/local-roots.js";
|
||||
import { readChannelAllowFromStore } from "../../../pairing/pairing-store.js";
|
||||
import type { resolveAgentRoute } from "../../../routing/resolve-route.js";
|
||||
import { jidToE164, normalizeE164 } from "../../../utils.js";
|
||||
import { resolveWhatsAppAccount } from "../../accounts.js";
|
||||
import { newConnectionId } from "../../reconnect.js";
|
||||
import { formatError } from "../../session.js";
|
||||
import { deliverWebReply } from "../deliver-reply.js";
|
||||
@@ -73,10 +74,11 @@ async function resolveWhatsAppCommandAuthorized(params: {
|
||||
return false;
|
||||
}
|
||||
|
||||
const configuredAllowFrom = params.cfg.channels?.whatsapp?.allowFrom ?? [];
|
||||
const account = resolveWhatsAppAccount({ cfg: params.cfg, accountId: params.msg.accountId });
|
||||
const dmPolicy = account.dmPolicy ?? "pairing";
|
||||
const configuredAllowFrom = account.allowFrom ?? [];
|
||||
const configuredGroupAllowFrom =
|
||||
params.cfg.channels?.whatsapp?.groupAllowFrom ??
|
||||
(configuredAllowFrom.length > 0 ? configuredAllowFrom : undefined);
|
||||
account.groupAllowFrom ?? (configuredAllowFrom.length > 0 ? configuredAllowFrom : undefined);
|
||||
|
||||
if (isGroup) {
|
||||
if (!configuredGroupAllowFrom || configuredGroupAllowFrom.length === 0) {
|
||||
@@ -88,11 +90,12 @@ async function resolveWhatsAppCommandAuthorized(params: {
|
||||
return normalizeAllowFromE164(configuredGroupAllowFrom).includes(senderE164);
|
||||
}
|
||||
|
||||
const storeAllowFrom = await readChannelAllowFromStore(
|
||||
"whatsapp",
|
||||
process.env,
|
||||
params.msg.accountId,
|
||||
).catch(() => []);
|
||||
const storeAllowFrom =
|
||||
dmPolicy === "allowlist"
|
||||
? []
|
||||
: await readChannelAllowFromStore("whatsapp", process.env, params.msg.accountId).catch(
|
||||
() => [],
|
||||
);
|
||||
const combinedAllowFrom = Array.from(
|
||||
new Set([...(configuredAllowFrom ?? []), ...storeAllowFrom]),
|
||||
);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
readAllowFromStoreMock,
|
||||
sendMessageMock,
|
||||
setAccessControlTestConfig,
|
||||
setupAccessControlTestHarness,
|
||||
@@ -108,4 +109,25 @@ describe("WhatsApp dmPolicy precedence", () => {
|
||||
const result = await checkUnauthorizedWorkDmSender();
|
||||
expectSilentlyBlocked(result);
|
||||
});
|
||||
|
||||
it("does not merge persisted pairing approvals in allowlist mode", async () => {
|
||||
setAccessControlTestConfig({
|
||||
channels: {
|
||||
whatsapp: {
|
||||
dmPolicy: "allowlist",
|
||||
accounts: {
|
||||
work: {
|
||||
allowFrom: ["+15559999999"],
|
||||
},
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
readAllowFromStoreMock.mockResolvedValue(["+15550001111"]);
|
||||
|
||||
const result = await checkUnauthorizedWorkDmSender();
|
||||
|
||||
expectSilentlyBlocked(result);
|
||||
expect(readAllowFromStoreMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -40,11 +40,10 @@ export async function checkInboundAccessControl(params: {
|
||||
});
|
||||
const dmPolicy = account.dmPolicy ?? "pairing";
|
||||
const configuredAllowFrom = account.allowFrom;
|
||||
const storeAllowFrom = await readChannelAllowFromStore(
|
||||
"whatsapp",
|
||||
process.env,
|
||||
account.accountId,
|
||||
).catch(() => []);
|
||||
const storeAllowFrom =
|
||||
dmPolicy === "allowlist"
|
||||
? []
|
||||
: await readChannelAllowFromStore("whatsapp", process.env, account.accountId).catch(() => []);
|
||||
// Without user config, default to self-only DM access so the owner can talk to themselves.
|
||||
const combinedAllowFrom = Array.from(
|
||||
new Set([...(configuredAllowFrom ?? []), ...storeAllowFrom]),
|
||||
|
||||
Reference in New Issue
Block a user