refactor(security): centralize channel allowlist auth policy

This commit is contained in:
Peter Steinberger
2026-02-26 13:06:27 +01:00
parent eac86c2081
commit 892a9c24b0
12 changed files with 137 additions and 90 deletions

View File

@@ -7,6 +7,7 @@ describe("irc inbound policy", () => {
configAllowFrom: ["owner"],
configGroupAllowFrom: [],
storeAllowList: ["paired-user"],
dmPolicy: "pairing",
});
expect(resolved.effectiveAllowFrom).toEqual(["owner", "paired-user"]);
@@ -17,6 +18,7 @@ describe("irc inbound policy", () => {
configAllowFrom: ["owner"],
configGroupAllowFrom: ["group-owner"],
storeAllowList: ["paired-user"],
dmPolicy: "pairing",
});
expect(resolved.effectiveGroupAllowFrom).toEqual(["group-owner"]);
@@ -27,6 +29,7 @@ describe("irc inbound policy", () => {
configAllowFrom: ["owner"],
configGroupAllowFrom: [],
storeAllowList: ["paired-user"],
dmPolicy: "pairing",
});
expect(resolved.effectiveGroupAllowFrom).toEqual([]);

View File

@@ -9,6 +9,7 @@ import {
resolveOutboundMediaUrls,
resolveAllowlistProviderRuntimeGroupPolicy,
resolveDefaultGroupPolicy,
resolveEffectiveAllowFromLists,
warnMissingProviderGroupPolicyFallbackOnce,
type OutboundReplyPayload,
type OpenClawConfig,
@@ -35,13 +36,19 @@ function resolveIrcEffectiveAllowlists(params: {
configAllowFrom: string[];
configGroupAllowFrom: string[];
storeAllowList: string[];
dmPolicy: string;
}): {
effectiveAllowFrom: string[];
effectiveGroupAllowFrom: string[];
} {
const effectiveAllowFrom = [...params.configAllowFrom, ...params.storeAllowList].filter(Boolean);
// Pairing-store entries are DM approvals and must not widen group sender authorization.
const effectiveGroupAllowFrom = [...params.configGroupAllowFrom].filter(Boolean);
const { effectiveAllowFrom, effectiveGroupAllowFrom } = resolveEffectiveAllowFromLists({
allowFrom: params.configAllowFrom,
groupAllowFrom: params.configGroupAllowFrom,
storeAllowFrom: params.storeAllowList,
dmPolicy: params.dmPolicy,
// IRC intentionally requires explicit groupAllowFrom; do not fallback to allowFrom.
groupAllowFromFallbackToAllowFrom: false,
});
return { effectiveAllowFrom, effectiveGroupAllowFrom };
}
@@ -141,6 +148,7 @@ export async function handleIrcInbound(params: {
configAllowFrom,
configGroupAllowFrom,
storeAllowList,
dmPolicy,
});
const allowTextCommands = core.channel.commands.shouldHandleTextCommands({

View File

@@ -0,0 +1,58 @@
import { resolveAllowlistMatchSimple, resolveEffectiveAllowFromLists } from "openclaw/plugin-sdk";
export function normalizeMattermostAllowEntry(entry: string): string {
const trimmed = entry.trim();
if (!trimmed) {
return "";
}
if (trimmed === "*") {
return "*";
}
return trimmed
.replace(/^(mattermost|user):/i, "")
.replace(/^@/, "")
.toLowerCase();
}
export function normalizeMattermostAllowList(entries: Array<string | number>): string[] {
const normalized = entries
.map((entry) => normalizeMattermostAllowEntry(String(entry)))
.filter(Boolean);
return Array.from(new Set(normalized));
}
export function resolveMattermostEffectiveAllowFromLists(params: {
allowFrom?: Array<string | number> | null;
groupAllowFrom?: Array<string | number> | null;
storeAllowFrom?: Array<string | number> | null;
dmPolicy?: string | null;
}): {
effectiveAllowFrom: string[];
effectiveGroupAllowFrom: string[];
} {
return resolveEffectiveAllowFromLists({
allowFrom: normalizeMattermostAllowList(params.allowFrom ?? []),
groupAllowFrom: normalizeMattermostAllowList(params.groupAllowFrom ?? []),
storeAllowFrom: normalizeMattermostAllowList(params.storeAllowFrom ?? []),
dmPolicy: params.dmPolicy,
});
}
export function isMattermostSenderAllowed(params: {
senderId: string;
senderName?: string;
allowFrom: string[];
allowNameMatching?: boolean;
}): boolean {
const allowFrom = params.allowFrom;
if (allowFrom.length === 0) {
return false;
}
const match = resolveAllowlistMatchSimple({
allowFrom,
senderId: normalizeMattermostAllowEntry(params.senderId),
senderName: params.senderName ? normalizeMattermostAllowEntry(params.senderName) : undefined,
allowNameMatching: params.allowNameMatching,
});
return match.allowed;
}

View File

@@ -1,5 +1,5 @@
import { describe, expect, it } from "vitest";
import { resolveMattermostEffectiveAllowFromLists } from "./monitor.js";
import { resolveMattermostEffectiveAllowFromLists } from "./monitor-auth.js";
describe("mattermost monitor authz", () => {
it("keeps DM allowlist merged with pairing-store entries", () => {

View File

@@ -18,7 +18,6 @@ import {
isDangerousNameMatchingEnabled,
resolveControlCommandGate,
resolveDmGroupAccessWithLists,
resolveEffectiveAllowFromLists,
resolveAllowlistProviderRuntimeGroupPolicy,
resolveDefaultGroupPolicy,
resolveChannelMediaMaxBytes,
@@ -38,6 +37,11 @@ import {
type MattermostPost,
type MattermostUser,
} from "./client.js";
import {
isMattermostSenderAllowed,
normalizeMattermostAllowList,
resolveMattermostEffectiveAllowFromLists,
} from "./monitor-auth.js";
import {
createDedupeCache,
formatInboundFromLabel,
@@ -132,68 +136,6 @@ function channelChatType(kind: ChatType): "direct" | "group" | "channel" {
return "channel";
}
function normalizeAllowEntry(entry: string): string {
const trimmed = entry.trim();
if (!trimmed) {
return "";
}
if (trimmed === "*") {
return "*";
}
return trimmed
.replace(/^(mattermost|user):/i, "")
.replace(/^@/, "")
.toLowerCase();
}
function normalizeAllowList(entries: Array<string | number>): string[] {
const normalized = entries.map((entry) => normalizeAllowEntry(String(entry))).filter(Boolean);
return Array.from(new Set(normalized));
}
export function resolveMattermostEffectiveAllowFromLists(params: {
allowFrom?: Array<string | number> | null;
groupAllowFrom?: Array<string | number> | null;
storeAllowFrom?: Array<string | number> | null;
dmPolicy?: string | null;
}): {
effectiveAllowFrom: string[];
effectiveGroupAllowFrom: string[];
} {
return resolveEffectiveAllowFromLists({
allowFrom: normalizeAllowList(params.allowFrom ?? []),
groupAllowFrom: normalizeAllowList(params.groupAllowFrom ?? []),
storeAllowFrom: normalizeAllowList(params.storeAllowFrom ?? []),
dmPolicy: params.dmPolicy,
});
}
function isSenderAllowed(params: {
senderId: string;
senderName?: string;
allowFrom: string[];
allowNameMatching?: boolean;
}): boolean {
const allowFrom = params.allowFrom;
if (allowFrom.length === 0) {
return false;
}
if (allowFrom.includes("*")) {
return true;
}
const normalizedSenderId = normalizeAllowEntry(params.senderId);
const normalizedSenderName = params.senderName ? normalizeAllowEntry(params.senderName) : "";
return allowFrom.some((entry) => {
if (entry === normalizedSenderId) {
return true;
}
if (params.allowNameMatching !== true) {
return false;
}
return normalizedSenderName ? entry === normalizedSenderName : false;
});
}
type MattermostMediaInfo = {
path: string;
contentType?: string;
@@ -418,7 +360,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
senderId;
const rawText = post.message?.trim() || "";
const dmPolicy = account.config.dmPolicy ?? "pairing";
const storeAllowFrom = normalizeAllowList(
const storeAllowFrom = normalizeMattermostAllowList(
dmPolicy === "allowlist"
? []
: await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []),
@@ -437,13 +379,13 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
const hasControlCommand = core.channel.text.hasControlCommand(rawText, cfg);
const isControlCommand = allowTextCommands && hasControlCommand;
const useAccessGroups = cfg.commands?.useAccessGroups !== false;
const senderAllowedForCommands = isSenderAllowed({
const senderAllowedForCommands = isMattermostSenderAllowed({
senderId,
senderName,
allowFrom: effectiveAllowFrom,
allowNameMatching,
});
const groupAllowedForCommands = isSenderAllowed({
const groupAllowedForCommands = isMattermostSenderAllowed({
senderId,
senderName,
allowFrom: effectiveGroupAllowFrom,
@@ -901,7 +843,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
// Enforce DM/group policy and allowlist checks (same as normal messages)
const dmPolicy = account.config.dmPolicy ?? "pairing";
const storeAllowFrom = normalizeAllowList(
const storeAllowFrom = normalizeMattermostAllowList(
dmPolicy === "allowlist"
? []
: await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []),
@@ -914,10 +856,10 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
groupAllowFrom: account.config.groupAllowFrom,
storeAllowFrom,
isSenderAllowed: (allowFrom) =>
isSenderAllowed({
isMattermostSenderAllowed({
senderId: userId,
senderName,
allowFrom: normalizeAllowList(allowFrom),
allowFrom: normalizeMattermostAllowList(allowFrom),
allowNameMatching,
}),
});

View File

@@ -9,6 +9,7 @@ import {
isDangerousNameMatchingEnabled,
resolveMentionGating,
formatAllowlistMatchMeta,
resolveEffectiveAllowFromLists,
type HistoryEntry,
} from "openclaw/plugin-sdk";
import {
@@ -136,7 +137,14 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
// Check DM policy for direct messages.
const dmAllowFrom = msteamsCfg?.allowFrom ?? [];
const configuredDmAllowFrom = dmAllowFrom.map((v) => String(v));
const effectiveDmAllowFrom = [...configuredDmAllowFrom, ...storedAllowFrom];
const groupAllowFrom = msteamsCfg?.groupAllowFrom;
const resolvedAllowFromLists = resolveEffectiveAllowFromLists({
allowFrom: configuredDmAllowFrom,
groupAllowFrom,
storeAllowFrom: storedAllowFrom,
dmPolicy,
});
const effectiveDmAllowFrom = resolvedAllowFromLists.effectiveAllowFrom;
if (isDirectMessage && msteamsCfg) {
const allowFrom = dmAllowFrom;
@@ -184,13 +192,8 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
!isDirectMessage && msteamsCfg
? (msteamsCfg.groupPolicy ?? defaultGroupPolicy ?? "allowlist")
: "disabled";
const groupAllowFrom =
!isDirectMessage && msteamsCfg
? (msteamsCfg.groupAllowFrom ??
(msteamsCfg.allowFrom && msteamsCfg.allowFrom.length > 0 ? msteamsCfg.allowFrom : []))
: [];
const effectiveGroupAllowFrom =
!isDirectMessage && msteamsCfg ? groupAllowFrom.map((v) => String(v)) : [];
!isDirectMessage && msteamsCfg ? resolvedAllowFromLists.effectiveGroupAllowFrom : [];
const teamId = activity.channelData?.team?.id;
const teamName = activity.channelData?.team?.name;
const channelName = activity.channelData?.channel?.name;

View File

@@ -55,6 +55,16 @@ describe("resolveGroupAllowFromSources", () => {
}),
).toEqual(["owner", "owner2"]);
});
it("can disable fallback to DM allowlist", () => {
expect(
resolveGroupAllowFromSources({
allowFrom: ["owner", "owner2"],
groupAllowFrom: [],
fallbackToAllowFrom: false,
}),
).toEqual([]);
});
});
describe("firstDefined", () => {

View File

@@ -12,10 +12,16 @@ export function mergeDmAllowFromSources(params: {
export function resolveGroupAllowFromSources(params: {
allowFrom?: Array<string | number>;
groupAllowFrom?: Array<string | number>;
fallbackToAllowFrom?: boolean;
}): string[] {
const scoped =
params.groupAllowFrom && params.groupAllowFrom.length > 0
const explicitGroupAllowFrom =
Array.isArray(params.groupAllowFrom) && params.groupAllowFrom.length > 0
? params.groupAllowFrom
: undefined;
const scoped = explicitGroupAllowFrom
? explicitGroupAllowFrom
: params.fallbackToAllowFrom === false
? []
: (params.allowFrom ?? []);
return scoped.map((value) => String(value).trim()).filter(Boolean);
}

View File

@@ -20,6 +20,7 @@ import {
resolveChannelGroupRequireMention,
} from "../../config/group-policy.js";
import { resolveAgentRoute } from "../../routing/resolve-route.js";
import { resolveEffectiveAllowFromLists } from "../../security/dm-policy-shared.js";
import { truncateUtf16Safe } from "../../utils.js";
import {
formatIMessageChatTarget,
@@ -138,14 +139,14 @@ export function resolveIMessageInboundDecision(params: {
}
const groupId = isGroup ? groupIdCandidate : undefined;
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.
const effectiveGroupAllowFrom = Array.from(new Set(params.groupAllowFrom))
.map((v) => String(v).trim())
.filter(Boolean);
const { effectiveAllowFrom: effectiveDmAllowFrom, effectiveGroupAllowFrom } =
resolveEffectiveAllowFromLists({
allowFrom: params.allowFrom,
groupAllowFrom: params.groupAllowFrom,
storeAllowFrom: params.storeAllowFrom,
dmPolicy: params.dmPolicy,
groupAllowFromFallbackToAllowFrom: false,
});
if (isGroup) {
if (params.groupPolicy === "disabled") {

View File

@@ -54,6 +54,17 @@ describe("security/dm-policy-shared", () => {
expect(lists.effectiveGroupAllowFrom).toEqual(["owner"]);
});
it("can keep group allowlist empty when fallback is disabled", () => {
const lists = resolveEffectiveAllowFromLists({
allowFrom: ["owner"],
groupAllowFrom: [],
storeAllowFrom: ["paired-user"],
groupAllowFromFallbackToAllowFrom: false,
});
expect(lists.effectiveAllowFrom).toEqual(["owner", "paired-user"]);
expect(lists.effectiveGroupAllowFrom).toEqual([]);
});
it("excludes storeAllowFrom when dmPolicy is allowlist", () => {
const lists = resolveEffectiveAllowFromLists({
allowFrom: ["+1111"],

View File

@@ -8,6 +8,7 @@ export function resolveEffectiveAllowFromLists(params: {
groupAllowFrom?: Array<string | number> | null;
storeAllowFrom?: Array<string | number> | null;
dmPolicy?: string | null;
groupAllowFromFallbackToAllowFrom?: boolean | null;
}): {
effectiveAllowFrom: string[];
effectiveGroupAllowFrom: string[];
@@ -27,6 +28,7 @@ export function resolveEffectiveAllowFromLists(params: {
resolveGroupAllowFromSources({
allowFrom,
groupAllowFrom,
fallbackToAllowFrom: params.groupAllowFromFallbackToAllowFrom ?? undefined,
}),
);
return { effectiveAllowFrom, effectiveGroupAllowFrom };
@@ -87,6 +89,7 @@ export function resolveDmGroupAccessWithLists(params: {
allowFrom?: Array<string | number> | null;
groupAllowFrom?: Array<string | number> | null;
storeAllowFrom?: Array<string | number> | null;
groupAllowFromFallbackToAllowFrom?: boolean | null;
isSenderAllowed: (allowFrom: string[]) => boolean;
}): {
decision: DmGroupAccessDecision;
@@ -99,6 +102,7 @@ export function resolveDmGroupAccessWithLists(params: {
groupAllowFrom: params.groupAllowFrom,
storeAllowFrom: params.storeAllowFrom,
dmPolicy: params.dmPolicy,
groupAllowFromFallbackToAllowFrom: params.groupAllowFromFallbackToAllowFrom,
});
const access = resolveDmGroupAccessDecision({
isGroup: params.isGroup,

View File

@@ -61,5 +61,6 @@ export async function buildTelegramMessageContextForTest(
groupConfig: { requireMention: false },
topicConfig: undefined,
})),
sendChatActionHandler: { sendChatAction: vi.fn() } as never,
});
}