From 5c5c032f4296ae4973c59c9d98949dc748adb991 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 18 Feb 2026 23:58:11 +0000 Subject: [PATCH] refactor(security): share DM allowlist state resolver --- src/commands/doctor-security.ts | 24 ++++------------ src/security/audit-channel.ts | 21 ++++---------- src/security/dm-policy-shared.test.ts | 31 +++++++++++++++++++++ src/security/dm-policy-shared.ts | 40 +++++++++++++++++++++++++++ 4 files changed, 83 insertions(+), 33 deletions(-) create mode 100644 src/security/dm-policy-shared.test.ts create mode 100644 src/security/dm-policy-shared.ts diff --git a/src/commands/doctor-security.ts b/src/commands/doctor-security.ts index 45fb85e4f7d..f58107e6838 100644 --- a/src/commands/doctor-security.ts +++ b/src/commands/doctor-security.ts @@ -5,8 +5,7 @@ import { formatCliCommand } from "../cli/command-format.js"; import type { OpenClawConfig, GatewayBindMode } from "../config/config.js"; import { resolveGatewayAuth } from "../gateway/auth.js"; import { isLoopbackHost, resolveGatewayBindHost } from "../gateway/net.js"; -import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; -import { normalizeStringEntries } from "../shared/string-normalization.js"; +import { resolveDmAllowState } from "../security/dm-policy-shared.js"; import { note } from "../terminal/note.js"; export async function noteSecurityWarnings(cfg: OpenClawConfig) { @@ -85,23 +84,12 @@ export async function noteSecurityWarnings(cfg: OpenClawConfig) { }) => { const dmPolicy = params.dmPolicy; const policyPath = params.policyPath ?? `${params.allowFromPath}policy`; - const configAllowFrom = normalizeStringEntries( - Array.isArray(params.allowFrom) ? params.allowFrom : undefined, - ); - const hasWildcard = configAllowFrom.includes("*"); - const storeAllowFrom = await readChannelAllowFromStore(params.provider).catch(() => []); - const normalizedCfg = configAllowFrom - .filter((v) => v !== "*") - .map((v) => (params.normalizeEntry ? params.normalizeEntry(v) : v)) - .map((v) => v.trim()) - .filter(Boolean); - const normalizedStore = storeAllowFrom - .map((v) => (params.normalizeEntry ? params.normalizeEntry(v) : v)) - .map((v) => v.trim()) - .filter(Boolean); - const allowCount = Array.from(new Set([...normalizedCfg, ...normalizedStore])).length; + const { hasWildcard, allowCount, isMultiUserDm } = await resolveDmAllowState({ + provider: params.provider, + allowFrom: params.allowFrom, + normalizeEntry: params.normalizeEntry, + }); const dmScope = cfg.session?.dmScope ?? "main"; - const isMultiUserDm = hasWildcard || allowCount > 1; if (dmPolicy === "open") { const allowFromPath = `${params.allowFromPath}allowFrom`; diff --git a/src/security/audit-channel.ts b/src/security/audit-channel.ts index 363d5dfe705..be70bb00b34 100644 --- a/src/security/audit-channel.ts +++ b/src/security/audit-channel.ts @@ -11,6 +11,7 @@ import type { OpenClawConfig } from "../config/config.js"; import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; import { normalizeStringEntries } from "../shared/string-normalization.js"; import type { SecurityAuditFinding, SecurityAuditSeverity } from "./audit.js"; +import { resolveDmAllowState } from "./dm-policy-shared.js"; function normalizeAllowFromList(list: Array | undefined | null): string[] { return normalizeStringEntries(Array.isArray(list) ? list : undefined); @@ -63,22 +64,12 @@ export async function collectChannelSecurityFindings(params: { normalizeEntry?: (raw: string) => string; }) => { const policyPath = input.policyPath ?? `${input.allowFromPath}policy`; - const configAllowFrom = normalizeAllowFromList(input.allowFrom); - const hasWildcard = configAllowFrom.includes("*"); + const { hasWildcard, isMultiUserDm } = await resolveDmAllowState({ + provider: input.provider, + allowFrom: input.allowFrom, + normalizeEntry: input.normalizeEntry, + }); const dmScope = params.cfg.session?.dmScope ?? "main"; - const storeAllowFrom = await readChannelAllowFromStore(input.provider).catch(() => []); - const normalizeEntry = input.normalizeEntry ?? ((value: string) => value); - const normalizedCfg = configAllowFrom - .filter((value) => value !== "*") - .map((value) => normalizeEntry(value)) - .map((value) => value.trim()) - .filter(Boolean); - const normalizedStore = storeAllowFrom - .map((value) => normalizeEntry(value)) - .map((value) => value.trim()) - .filter(Boolean); - const allowCount = Array.from(new Set([...normalizedCfg, ...normalizedStore])).length; - const isMultiUserDm = hasWildcard || allowCount > 1; if (input.dmPolicy === "open") { const allowFromKey = `${input.allowFromPath}allowFrom`; diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts new file mode 100644 index 00000000000..13acf939aba --- /dev/null +++ b/src/security/dm-policy-shared.test.ts @@ -0,0 +1,31 @@ +import { describe, expect, it } from "vitest"; +import { resolveDmAllowState } from "./dm-policy-shared.js"; + +describe("security/dm-policy-shared", () => { + it("normalizes config + store allow entries and counts distinct senders", async () => { + const state = await resolveDmAllowState({ + provider: "telegram", + allowFrom: [" * ", " alice ", "ALICE", "bob"], + normalizeEntry: (value) => value.toLowerCase(), + readStore: async () => [" Bob ", "carol", ""], + }); + expect(state.configAllowFrom).toEqual(["*", "alice", "ALICE", "bob"]); + expect(state.hasWildcard).toBe(true); + expect(state.allowCount).toBe(3); + expect(state.isMultiUserDm).toBe(true); + }); + + it("handles empty allowlists and store failures", async () => { + const state = await resolveDmAllowState({ + provider: "slack", + allowFrom: undefined, + readStore: async () => { + throw new Error("offline"); + }, + }); + expect(state.configAllowFrom).toEqual([]); + expect(state.hasWildcard).toBe(false); + expect(state.allowCount).toBe(0); + expect(state.isMultiUserDm).toBe(false); + }); +}); diff --git a/src/security/dm-policy-shared.ts b/src/security/dm-policy-shared.ts new file mode 100644 index 00000000000..b9338fdac75 --- /dev/null +++ b/src/security/dm-policy-shared.ts @@ -0,0 +1,40 @@ +import type { ChannelId } from "../channels/plugins/types.js"; +import { readChannelAllowFromStore } from "../pairing/pairing-store.js"; +import { normalizeStringEntries } from "../shared/string-normalization.js"; + +export async function resolveDmAllowState(params: { + provider: ChannelId; + allowFrom?: Array | null; + normalizeEntry?: (raw: string) => string; + readStore?: (provider: ChannelId) => Promise; +}): Promise<{ + configAllowFrom: string[]; + hasWildcard: boolean; + allowCount: number; + isMultiUserDm: boolean; +}> { + const configAllowFrom = normalizeStringEntries( + Array.isArray(params.allowFrom) ? params.allowFrom : undefined, + ); + const hasWildcard = configAllowFrom.includes("*"); + const storeAllowFrom = await (params.readStore ?? readChannelAllowFromStore)( + params.provider, + ).catch(() => []); + const normalizeEntry = params.normalizeEntry ?? ((value: string) => value); + const normalizedCfg = configAllowFrom + .filter((value) => value !== "*") + .map((value) => normalizeEntry(value)) + .map((value) => value.trim()) + .filter(Boolean); + const normalizedStore = storeAllowFrom + .map((value) => normalizeEntry(value)) + .map((value) => value.trim()) + .filter(Boolean); + const allowCount = Array.from(new Set([...normalizedCfg, ...normalizedStore])).length; + return { + configAllowFrom, + hasWildcard, + allowCount, + isMultiUserDm: hasWildcard || allowCount > 1, + }; +}