Skip to content

Commit 892a9c2

Browse files
steipetesteipete
committed
refactor(security): centralize channel allowlist auth policy
1 parent eac86c2 commit 892a9c2

12 files changed

Lines changed: 137 additions & 90 deletions

File tree

‎extensions/irc/src/inbound.policy.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ describe("irc inbound policy", () => {
77
configAllowFrom: ["owner"],
88
configGroupAllowFrom: [],
99
storeAllowList: ["paired-user"],
10+
dmPolicy: "pairing",
1011
});
1112

1213
expect(resolved.effectiveAllowFrom).toEqual(["owner", "paired-user"]);
@@ -17,6 +18,7 @@ describe("irc inbound policy", () => {
1718
configAllowFrom: ["owner"],
1819
configGroupAllowFrom: ["group-owner"],
1920
storeAllowList: ["paired-user"],
21+
dmPolicy: "pairing",
2022
});
2123

2224
expect(resolved.effectiveGroupAllowFrom).toEqual(["group-owner"]);
@@ -27,6 +29,7 @@ describe("irc inbound policy", () => {
2729
configAllowFrom: ["owner"],
2830
configGroupAllowFrom: [],
2931
storeAllowList: ["paired-user"],
32+
dmPolicy: "pairing",
3033
});
3134

3235
expect(resolved.effectiveGroupAllowFrom).toEqual([]);

‎extensions/irc/src/inbound.ts‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
resolveOutboundMediaUrls,
1010
resolveAllowlistProviderRuntimeGroupPolicy,
1111
resolveDefaultGroupPolicy,
12+
resolveEffectiveAllowFromLists,
1213
warnMissingProviderGroupPolicyFallbackOnce,
1314
type OutboundReplyPayload,
1415
type OpenClawConfig,
@@ -35,13 +36,19 @@ function resolveIrcEffectiveAllowlists(params: {
3536
configAllowFrom: string[];
3637
configGroupAllowFrom: string[];
3738
storeAllowList: string[];
39+
dmPolicy: string;
3840
}): {
3941
effectiveAllowFrom: string[];
4042
effectiveGroupAllowFrom: string[];
4143
} {
42-
const effectiveAllowFrom = [...params.configAllowFrom, ...params.storeAllowList].filter(Boolean);
43-
// Pairing-store entries are DM approvals and must not widen group sender authorization.
44-
const effectiveGroupAllowFrom = [...params.configGroupAllowFrom].filter(Boolean);
44+
const { effectiveAllowFrom, effectiveGroupAllowFrom } = resolveEffectiveAllowFromLists({
45+
allowFrom: params.configAllowFrom,
46+
groupAllowFrom: params.configGroupAllowFrom,
47+
storeAllowFrom: params.storeAllowList,
48+
dmPolicy: params.dmPolicy,
49+
// IRC intentionally requires explicit groupAllowFrom; do not fallback to allowFrom.
50+
groupAllowFromFallbackToAllowFrom: false,
51+
});
4552
return { effectiveAllowFrom, effectiveGroupAllowFrom };
4653
}
4754

@@ -141,6 +148,7 @@ export async function handleIrcInbound(params: {
141148
configAllowFrom,
142149
configGroupAllowFrom,
143150
storeAllowList,
151+
dmPolicy,
144152
});
145153

146154
const allowTextCommands = core.channel.commands.shouldHandleTextCommands({

‎extensions/mattermost/src/mattermost/monitor-auth.ts‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import { resolveAllowlistMatchSimple, resolveEffectiveAllowFromLists } from "openclaw/plugin-sdk";
2+
3+
export function normalizeMattermostAllowEntry(entry: string): string {
4+
const trimmed = entry.trim();
5+
if (!trimmed) {
6+
return "";
7+
}
8+
if (trimmed === "*") {
9+
return "*";
10+
}
11+
return trimmed
12+
.replace(/^(mattermost|user):/i, "")
13+
.replace(/^@/, "")
14+
.toLowerCase();
15+
}
16+
17+
export function normalizeMattermostAllowList(entries: Array<string | number>): string[] {
18+
const normalized = entries
19+
.map((entry) => normalizeMattermostAllowEntry(String(entry)))
20+
.filter(Boolean);
21+
return Array.from(new Set(normalized));
22+
}
23+
24+
export function resolveMattermostEffectiveAllowFromLists(params: {
25+
allowFrom?: Array<string | number> | null;
26+
groupAllowFrom?: Array<string | number> | null;
27+
storeAllowFrom?: Array<string | number> | null;
28+
dmPolicy?: string | null;
29+
}): {
30+
effectiveAllowFrom: string[];
31+
effectiveGroupAllowFrom: string[];
32+
} {
33+
return resolveEffectiveAllowFromLists({
34+
allowFrom: normalizeMattermostAllowList(params.allowFrom ?? []),
35+
groupAllowFrom: normalizeMattermostAllowList(params.groupAllowFrom ?? []),
36+
storeAllowFrom: normalizeMattermostAllowList(params.storeAllowFrom ?? []),
37+
dmPolicy: params.dmPolicy,
38+
});
39+
}
40+
41+
export function isMattermostSenderAllowed(params: {
42+
senderId: string;
43+
senderName?: string;
44+
allowFrom: string[];
45+
allowNameMatching?: boolean;
46+
}): boolean {
47+
const allowFrom = params.allowFrom;
48+
if (allowFrom.length === 0) {
49+
return false;
50+
}
51+
const match = resolveAllowlistMatchSimple({
52+
allowFrom,
53+
senderId: normalizeMattermostAllowEntry(params.senderId),
54+
senderName: params.senderName ? normalizeMattermostAllowEntry(params.senderName) : undefined,
55+
allowNameMatching: params.allowNameMatching,
56+
});
57+
return match.allowed;
58+
}

‎extensions/mattermost/src/mattermost/monitor.authz.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { describe, expect, it } from "vitest";
2-
import { resolveMattermostEffectiveAllowFromLists } from "./monitor.js";
2+
import { resolveMattermostEffectiveAllowFromLists } from "./monitor-auth.js";
33

44
describe("mattermost monitor authz", () => {
55
it("keeps DM allowlist merged with pairing-store entries", () => {

‎extensions/mattermost/src/mattermost/monitor.ts‎

Lines changed: 11 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@ import {
1818
isDangerousNameMatchingEnabled,
1919
resolveControlCommandGate,
2020
resolveDmGroupAccessWithLists,
21-
resolveEffectiveAllowFromLists,
2221
resolveAllowlistProviderRuntimeGroupPolicy,
2322
resolveDefaultGroupPolicy,
2423
resolveChannelMediaMaxBytes,
@@ -38,6 +37,11 @@ import {
3837
type MattermostPost,
3938
type MattermostUser,
4039
} from "./client.js";
40+
import {
41+
isMattermostSenderAllowed,
42+
normalizeMattermostAllowList,
43+
resolveMattermostEffectiveAllowFromLists,
44+
} from "./monitor-auth.js";
4145
import {
4246
createDedupeCache,
4347
formatInboundFromLabel,
@@ -132,68 +136,6 @@ function channelChatType(kind: ChatType): "direct" | "group" | "channel" {
132136
return "channel";
133137
}
134138

135-
function normalizeAllowEntry(entry: string): string {
136-
const trimmed = entry.trim();
137-
if (!trimmed) {
138-
return "";
139-
}
140-
if (trimmed === "*") {
141-
return "*";
142-
}
143-
return trimmed
144-
.replace(/^(mattermost|user):/i, "")
145-
.replace(/^@/, "")
146-
.toLowerCase();
147-
}
148-
149-
function normalizeAllowList(entries: Array<string | number>): string[] {
150-
const normalized = entries.map((entry) => normalizeAllowEntry(String(entry))).filter(Boolean);
151-
return Array.from(new Set(normalized));
152-
}
153-
154-
export function resolveMattermostEffectiveAllowFromLists(params: {
155-
allowFrom?: Array<string | number> | null;
156-
groupAllowFrom?: Array<string | number> | null;
157-
storeAllowFrom?: Array<string | number> | null;
158-
dmPolicy?: string | null;
159-
}): {
160-
effectiveAllowFrom: string[];
161-
effectiveGroupAllowFrom: string[];
162-
} {
163-
return resolveEffectiveAllowFromLists({
164-
allowFrom: normalizeAllowList(params.allowFrom ?? []),
165-
groupAllowFrom: normalizeAllowList(params.groupAllowFrom ?? []),
166-
storeAllowFrom: normalizeAllowList(params.storeAllowFrom ?? []),
167-
dmPolicy: params.dmPolicy,
168-
});
169-
}
170-
171-
function isSenderAllowed(params: {
172-
senderId: string;
173-
senderName?: string;
174-
allowFrom: string[];
175-
allowNameMatching?: boolean;
176-
}): boolean {
177-
const allowFrom = params.allowFrom;
178-
if (allowFrom.length === 0) {
179-
return false;
180-
}
181-
if (allowFrom.includes("*")) {
182-
return true;
183-
}
184-
const normalizedSenderId = normalizeAllowEntry(params.senderId);
185-
const normalizedSenderName = params.senderName ? normalizeAllowEntry(params.senderName) : "";
186-
return allowFrom.some((entry) => {
187-
if (entry === normalizedSenderId) {
188-
return true;
189-
}
190-
if (params.allowNameMatching !== true) {
191-
return false;
192-
}
193-
return normalizedSenderName ? entry === normalizedSenderName : false;
194-
});
195-
}
196-
197139
type MattermostMediaInfo = {
198140
path: string;
199141
contentType?: string;
@@ -418,7 +360,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
418360
senderId;
419361
const rawText = post.message?.trim() || "";
420362
const dmPolicy = account.config.dmPolicy ?? "pairing";
421-
const storeAllowFrom = normalizeAllowList(
363+
const storeAllowFrom = normalizeMattermostAllowList(
422364
dmPolicy === "allowlist"
423365
? []
424366
: await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []),
@@ -437,13 +379,13 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
437379
const hasControlCommand = core.channel.text.hasControlCommand(rawText, cfg);
438380
const isControlCommand = allowTextCommands && hasControlCommand;
439381
const useAccessGroups = cfg.commands?.useAccessGroups !== false;
440-
const senderAllowedForCommands = isSenderAllowed({
382+
const senderAllowedForCommands = isMattermostSenderAllowed({
441383
senderId,
442384
senderName,
443385
allowFrom: effectiveAllowFrom,
444386
allowNameMatching,
445387
});
446-
const groupAllowedForCommands = isSenderAllowed({
388+
const groupAllowedForCommands = isMattermostSenderAllowed({
447389
senderId,
448390
senderName,
449391
allowFrom: effectiveGroupAllowFrom,
@@ -901,7 +843,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
901843

902844
// Enforce DM/group policy and allowlist checks (same as normal messages)
903845
const dmPolicy = account.config.dmPolicy ?? "pairing";
904-
const storeAllowFrom = normalizeAllowList(
846+
const storeAllowFrom = normalizeMattermostAllowList(
905847
dmPolicy === "allowlist"
906848
? []
907849
: await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []),
@@ -914,10 +856,10 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {}
914856
groupAllowFrom: account.config.groupAllowFrom,
915857
storeAllowFrom,
916858
isSenderAllowed: (allowFrom) =>
917-
isSenderAllowed({
859+
isMattermostSenderAllowed({
918860
senderId: userId,
919861
senderName,
920-
allowFrom: normalizeAllowList(allowFrom),
862+
allowFrom: normalizeMattermostAllowList(allowFrom),
921863
allowNameMatching,
922864
}),
923865
});

‎extensions/msteams/src/monitor-handler/message-handler.ts‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {
99
isDangerousNameMatchingEnabled,
1010
resolveMentionGating,
1111
formatAllowlistMatchMeta,
12+
resolveEffectiveAllowFromLists,
1213
type HistoryEntry,
1314
} from "openclaw/plugin-sdk";
1415
import {
@@ -136,7 +137,14 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
136137
// Check DM policy for direct messages.
137138
const dmAllowFrom = msteamsCfg?.allowFrom ?? [];
138139
const configuredDmAllowFrom = dmAllowFrom.map((v) => String(v));
139-
const effectiveDmAllowFrom = [...configuredDmAllowFrom, ...storedAllowFrom];
140+
const groupAllowFrom = msteamsCfg?.groupAllowFrom;
141+
const resolvedAllowFromLists = resolveEffectiveAllowFromLists({
142+
allowFrom: configuredDmAllowFrom,
143+
groupAllowFrom,
144+
storeAllowFrom: storedAllowFrom,
145+
dmPolicy,
146+
});
147+
const effectiveDmAllowFrom = resolvedAllowFromLists.effectiveAllowFrom;
140148
if (isDirectMessage && msteamsCfg) {
141149
const allowFrom = dmAllowFrom;
142150

@@ -184,13 +192,8 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) {
184192
!isDirectMessage && msteamsCfg
185193
? (msteamsCfg.groupPolicy ?? defaultGroupPolicy ?? "allowlist")
186194
: "disabled";
187-
const groupAllowFrom =
188-
!isDirectMessage && msteamsCfg
189-
? (msteamsCfg.groupAllowFrom ??
190-
(msteamsCfg.allowFrom && msteamsCfg.allowFrom.length > 0 ? msteamsCfg.allowFrom : []))
191-
: [];
192195
const effectiveGroupAllowFrom =
193-
!isDirectMessage && msteamsCfg ? groupAllowFrom.map((v) => String(v)) : [];
196+
!isDirectMessage && msteamsCfg ? resolvedAllowFromLists.effectiveGroupAllowFrom : [];
194197
const teamId = activity.channelData?.team?.id;
195198
const teamName = activity.channelData?.team?.name;
196199
const channelName = activity.channelData?.channel?.name;

‎src/channels/allow-from.test.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ describe("resolveGroupAllowFromSources", () => {
5555
}),
5656
).toEqual(["owner", "owner2"]);
5757
});
58+
59+
it("can disable fallback to DM allowlist", () => {
60+
expect(
61+
resolveGroupAllowFromSources({
62+
allowFrom: ["owner", "owner2"],
63+
groupAllowFrom: [],
64+
fallbackToAllowFrom: false,
65+
}),
66+
).toEqual([]);
67+
});
5868
});
5969

6070
describe("firstDefined", () => {

‎src/channels/allow-from.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,16 @@ export function mergeDmAllowFromSources(params: {
1212
export function resolveGroupAllowFromSources(params: {
1313
allowFrom?: Array<string | number>;
1414
groupAllowFrom?: Array<string | number>;
15+
fallbackToAllowFrom?: boolean;
1516
}): string[] {
16-
const scoped =
17-
params.groupAllowFrom && params.groupAllowFrom.length > 0
17+
const explicitGroupAllowFrom =
18+
Array.isArray(params.groupAllowFrom) && params.groupAllowFrom.length > 0
1819
? params.groupAllowFrom
20+
: undefined;
21+
const scoped = explicitGroupAllowFrom
22+
? explicitGroupAllowFrom
23+
: params.fallbackToAllowFrom === false
24+
? []
1925
: (params.allowFrom ?? []);
2026
return scoped.map((value) => String(value).trim()).filter(Boolean);
2127
}

‎src/imessage/monitor/inbound-processing.ts‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
resolveChannelGroupRequireMention,
2121
} from "../../config/group-policy.js";
2222
import { resolveAgentRoute } from "../../routing/resolve-route.js";
23+
import { resolveEffectiveAllowFromLists } from "../../security/dm-policy-shared.js";
2324
import { truncateUtf16Safe } from "../../utils.js";
2425
import {
2526
formatIMessageChatTarget,
@@ -138,14 +139,14 @@ export function resolveIMessageInboundDecision(params: {
138139
}
139140

140141
const groupId = isGroup ? groupIdCandidate : undefined;
141-
const storeAllowFrom = params.dmPolicy === "allowlist" ? [] : params.storeAllowFrom;
142-
const effectiveDmAllowFrom = Array.from(new Set([...params.allowFrom, ...storeAllowFrom]))
143-
.map((v) => String(v).trim())
144-
.filter(Boolean);
145-
// Keep DM pairing-store authorization scoped to DMs; group access must come from explicit group allowlist config.
146-
const effectiveGroupAllowFrom = Array.from(new Set(params.groupAllowFrom))
147-
.map((v) => String(v).trim())
148-
.filter(Boolean);
142+
const { effectiveAllowFrom: effectiveDmAllowFrom, effectiveGroupAllowFrom } =
143+
resolveEffectiveAllowFromLists({
144+
allowFrom: params.allowFrom,
145+
groupAllowFrom: params.groupAllowFrom,
146+
storeAllowFrom: params.storeAllowFrom,
147+
dmPolicy: params.dmPolicy,
148+
groupAllowFromFallbackToAllowFrom: false,
149+
});
149150

150151
if (isGroup) {
151152
if (params.groupPolicy === "disabled") {

‎src/security/dm-policy-shared.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,17 @@ describe("security/dm-policy-shared", () => {
5454
expect(lists.effectiveGroupAllowFrom).toEqual(["owner"]);
5555
});
5656

57+
it("can keep group allowlist empty when fallback is disabled", () => {
58+
const lists = resolveEffectiveAllowFromLists({
59+
allowFrom: ["owner"],
60+
groupAllowFrom: [],
61+
storeAllowFrom: ["paired-user"],
62+
groupAllowFromFallbackToAllowFrom: false,
63+
});
64+
expect(lists.effectiveAllowFrom).toEqual(["owner", "paired-user"]);
65+
expect(lists.effectiveGroupAllowFrom).toEqual([]);
66+
});
67+
5768
it("excludes storeAllowFrom when dmPolicy is allowlist", () => {
5869
const lists = resolveEffectiveAllowFromLists({
5970
allowFrom: ["+1111"],

0 commit comments

Comments
 (0)