From cd80c7e7ff5a7e7518888a39edfdf8507be8b10a Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 26 Feb 2026 17:47:51 +0100 Subject: [PATCH] refactor: unify dm policy store reads and reason codes --- .../bluebubbles/src/monitor-processing.ts | 26 +++--- extensions/irc/src/inbound.ts | 10 ++- .../matrix/src/matrix/monitor/handler.ts | 10 ++- .../mattermost/src/mattermost/monitor.ts | 26 +++--- .../src/monitor-handler/message-handler.ts | 10 ++- extensions/nextcloud-talk/src/inbound.ts | 10 ++- scripts/check-no-pairing-store-group-auth.mjs | 20 ++++- src/discord/monitor/agent-components.ts | 8 +- src/discord/monitor/listeners.ts | 14 +-- .../monitor/message-handler.preflight.ts | 8 +- src/discord/monitor/native-command.ts | 8 +- src/imessage/monitor/inbound-processing.ts | 13 +-- src/plugin-sdk/index.ts | 3 + src/security/dm-policy-channel-smoke.test.ts | 3 +- src/security/dm-policy-shared.test.ts | 35 ++++++++ src/security/dm-policy-shared.ts | 88 ++++++++++++++++--- src/signal/monitor/event-handler.ts | 19 ++-- src/slack/monitor/auth.ts | 8 +- src/slack/monitor/slash.ts | 10 ++- src/web/auto-reply/monitor/process-message.ts | 12 +-- src/web/inbound/access-control.ts | 10 ++- 21 files changed, 259 insertions(+), 92 deletions(-) diff --git a/extensions/bluebubbles/src/monitor-processing.ts b/extensions/bluebubbles/src/monitor-processing.ts index f25e47d50e6..936308d8b9e 100644 --- a/extensions/bluebubbles/src/monitor-processing.ts +++ b/extensions/bluebubbles/src/monitor-processing.ts @@ -1,10 +1,12 @@ import type { OpenClawConfig } from "openclaw/plugin-sdk"; import { + DM_GROUP_ACCESS_REASON, createReplyPrefixOptions, evictOldHistoryKeys, logAckFailure, logInboundDrop, logTypingFailure, + readStoreAllowFromForDmPolicy, recordPendingHistoryEntryIfEnabled, resolveAckReaction, resolveDmGroupAccessWithLists, @@ -500,9 +502,11 @@ export async function processMessage( const dmPolicy = account.config.dmPolicy ?? "pairing"; const groupPolicy = account.config.groupPolicy ?? "allowlist"; - const storeAllowFrom = await core.channel.pairing - .readAllowFromStore("bluebubbles") - .catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "bluebubbles", + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }); const accessDecision = resolveDmGroupAccessWithLists({ isGroup, dmPolicy, @@ -530,7 +534,7 @@ export async function processMessage( if (accessDecision.decision !== "allow") { if (isGroup) { - if (accessDecision.reason === "groupPolicy=disabled") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED) { logVerbose(core, runtime, "Blocked BlueBubbles group message (groupPolicy=disabled)"); logGroupAllowlistHint({ runtime, @@ -541,7 +545,7 @@ export async function processMessage( }); return; } - if (accessDecision.reason === "groupPolicy=allowlist (empty allowlist)") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST) { logVerbose(core, runtime, "Blocked BlueBubbles group message (no allowlist)"); logGroupAllowlistHint({ runtime, @@ -552,7 +556,7 @@ export async function processMessage( }); return; } - if (accessDecision.reason === "groupPolicy=allowlist (not allowlisted)") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED) { logVerbose( core, runtime, @@ -575,7 +579,7 @@ export async function processMessage( return; } - if (accessDecision.reason === "dmPolicy=disabled") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.DM_POLICY_DISABLED) { logVerbose(core, runtime, `Blocked BlueBubbles DM from ${message.senderId}`); logVerbose(core, runtime, `drop: dmPolicy disabled sender=${message.senderId}`); return; @@ -1382,9 +1386,11 @@ export async function processReaction( const dmPolicy = account.config.dmPolicy ?? "pairing"; const groupPolicy = account.config.groupPolicy ?? "allowlist"; - const storeAllowFrom = await core.channel.pairing - .readAllowFromStore("bluebubbles") - .catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "bluebubbles", + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }); const accessDecision = resolveDmGroupAccessWithLists({ isGroup: reaction.isGroup, dmPolicy, diff --git a/extensions/irc/src/inbound.ts b/extensions/irc/src/inbound.ts index 2efe735f228..29d2327112f 100644 --- a/extensions/irc/src/inbound.ts +++ b/extensions/irc/src/inbound.ts @@ -5,6 +5,7 @@ import { formatTextWithAttachmentLinks, logInboundDrop, isDangerousNameMatchingEnabled, + readStoreAllowFromForDmPolicy, resolveControlCommandGate, resolveOutboundMediaUrls, resolveAllowlistProviderRuntimeGroupPolicy, @@ -120,10 +121,11 @@ export async function handleIrcInbound(params: { const configAllowFrom = normalizeIrcAllowlist(account.config.allowFrom); const configGroupAllowFrom = normalizeIrcAllowlist(account.config.groupAllowFrom); - const storeAllowFrom = - dmPolicy === "allowlist" - ? [] - : await core.channel.pairing.readAllowFromStore(CHANNEL_ID).catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: CHANNEL_ID, + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }); const storeAllowList = normalizeIrcAllowlist(storeAllowFrom); const groupMatch = resolveIrcGroupMatch({ diff --git a/extensions/matrix/src/matrix/monitor/handler.ts b/extensions/matrix/src/matrix/monitor/handler.ts index 8907c9c033e..0d864850775 100644 --- a/extensions/matrix/src/matrix/monitor/handler.ts +++ b/extensions/matrix/src/matrix/monitor/handler.ts @@ -5,6 +5,7 @@ import { formatAllowlistMatchMeta, logInboundDrop, logTypingFailure, + readStoreAllowFromForDmPolicy, resolveControlCommandGate, type PluginRuntime, type RuntimeEnv, @@ -213,10 +214,11 @@ export function createMatrixRoomMessageHandler(params: MatrixMonitorHandlerParam } const senderName = await getMemberDisplayName(roomId, senderId); - const storeAllowFrom = - dmPolicy === "allowlist" - ? [] - : await core.channel.pairing.readAllowFromStore("matrix").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "matrix", + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }); const effectiveAllowFrom = normalizeMatrixAllowList([...allowFrom, ...storeAllowFrom]); const groupAllowFrom = cfg.channels?.matrix?.groupAllowFrom ?? []; const effectiveGroupAllowFrom = normalizeMatrixAllowList(groupAllowFrom); diff --git a/extensions/mattermost/src/mattermost/monitor.ts b/extensions/mattermost/src/mattermost/monitor.ts index 4aeca9eb212..0c6b9f6febc 100644 --- a/extensions/mattermost/src/mattermost/monitor.ts +++ b/extensions/mattermost/src/mattermost/monitor.ts @@ -7,6 +7,7 @@ import type { } from "openclaw/plugin-sdk"; import { buildAgentMediaPayload, + DM_GROUP_ACCESS_REASON, createReplyPrefixOptions, createTypingCallbacks, logInboundDrop, @@ -17,6 +18,7 @@ import { recordPendingHistoryEntryIfEnabled, isDangerousNameMatchingEnabled, resolveControlCommandGate, + readStoreAllowFromForDmPolicy, resolveDmGroupAccessWithLists, resolveAllowlistProviderRuntimeGroupPolicy, resolveDefaultGroupPolicy, @@ -358,9 +360,11 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} account.config.groupAllowFrom ?? [], ); const storeAllowFrom = normalizeMattermostAllowList( - dmPolicy === "allowlist" - ? [] - : await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []), + await readStoreAllowFromForDmPolicy({ + provider: "mattermost", + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }), ); const accessDecision = resolveDmGroupAccessWithLists({ isGroup: kind !== "direct", @@ -415,7 +419,7 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} if (accessDecision.decision !== "allow") { if (kind === "direct") { - if (accessDecision.reason === "dmPolicy=disabled") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.DM_POLICY_DISABLED) { logVerboseMessage(`mattermost: drop dm (dmPolicy=disabled sender=${senderId})`); return; } @@ -447,15 +451,15 @@ export async function monitorMattermostProvider(opts: MonitorMattermostOpts = {} logVerboseMessage(`mattermost: drop dm sender=${senderId} (dmPolicy=${dmPolicy})`); return; } - if (accessDecision.reason === "groupPolicy=disabled") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED) { logVerboseMessage("mattermost: drop group message (groupPolicy=disabled)"); return; } - if (accessDecision.reason === "groupPolicy=allowlist (empty allowlist)") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST) { logVerboseMessage("mattermost: drop group message (no group allowlist)"); return; } - if (accessDecision.reason === "groupPolicy=allowlist (not allowlisted)") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED) { logVerboseMessage(`mattermost: drop group sender=${senderId} (not in groupAllowFrom)`); return; } @@ -856,9 +860,11 @@ 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 = normalizeMattermostAllowList( - dmPolicy === "allowlist" - ? [] - : await core.channel.pairing.readAllowFromStore("mattermost").catch(() => []), + await readStoreAllowFromForDmPolicy({ + provider: "mattermost", + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }), ); const reactionAccess = resolveDmGroupAccessWithLists({ isGroup: kind !== "direct", diff --git a/extensions/msteams/src/monitor-handler/message-handler.ts b/extensions/msteams/src/monitor-handler/message-handler.ts index bba6d16acb5..e420892b564 100644 --- a/extensions/msteams/src/monitor-handler/message-handler.ts +++ b/extensions/msteams/src/monitor-handler/message-handler.ts @@ -7,6 +7,7 @@ import { resolveControlCommandGate, resolveDefaultGroupPolicy, isDangerousNameMatchingEnabled, + readStoreAllowFromForDmPolicy, resolveMentionGating, formatAllowlistMatchMeta, resolveEffectiveAllowFromLists, @@ -128,10 +129,11 @@ export function createMSTeamsMessageHandler(deps: MSTeamsMessageHandlerDeps) { const senderName = from.name ?? from.id; const senderId = from.aadObjectId ?? from.id; const dmPolicy = msteamsCfg?.dmPolicy ?? "pairing"; - const storedAllowFrom = - dmPolicy === "allowlist" - ? [] - : await core.channel.pairing.readAllowFromStore("msteams").catch(() => []); + const storedAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "msteams", + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }); const useAccessGroups = cfg.commands?.useAccessGroups !== false; // Check DM policy for direct messages. diff --git a/extensions/nextcloud-talk/src/inbound.ts b/extensions/nextcloud-talk/src/inbound.ts index 526249aa977..a1f4acef109 100644 --- a/extensions/nextcloud-talk/src/inbound.ts +++ b/extensions/nextcloud-talk/src/inbound.ts @@ -4,6 +4,7 @@ import { createReplyPrefixOptions, formatTextWithAttachmentLinks, logInboundDrop, + readStoreAllowFromForDmPolicy, resolveControlCommandGate, resolveOutboundMediaUrls, resolveAllowlistProviderRuntimeGroupPolicy, @@ -96,10 +97,11 @@ export async function handleNextcloudTalkInbound(params: { const configAllowFrom = normalizeNextcloudTalkAllowlist(account.config.allowFrom); const configGroupAllowFrom = normalizeNextcloudTalkAllowlist(account.config.groupAllowFrom); - const storeAllowFrom = - dmPolicy === "allowlist" - ? [] - : await core.channel.pairing.readAllowFromStore(CHANNEL_ID).catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: CHANNEL_ID, + dmPolicy, + readStore: (provider) => core.channel.pairing.readAllowFromStore(provider), + }); const storeAllowList = normalizeNextcloudTalkAllowlist(storeAllowFrom); const roomMatch = resolveNextcloudTalkRoomMatch({ diff --git a/scripts/check-no-pairing-store-group-auth.mjs b/scripts/check-no-pairing-store-group-auth.mjs index e4fcc0e4fad..316411c460e 100644 --- a/scripts/check-no-pairing-store-group-auth.mjs +++ b/scripts/check-no-pairing-store-group-auth.mjs @@ -19,6 +19,11 @@ const allowedFiles = new Set([ const storeIdentifierRe = /^(?:storeAllowFrom|storedAllowFrom|storeAllowList)$/i; const groupNameRe = /(?:groupAllowFrom|effectiveGroupAllowFrom|groupAllowed|groupAllow|groupAuth|groupSender)/i; +const storeSourceCallNames = new Set([ + "readChannelAllowFromStore", + "readChannelAllowFromStoreSync", + "readStoreAllowFromForDmPolicy", +]); const allowedResolverCallNames = new Set([ "resolveEffectiveAllowFromLists", "resolveDmGroupAccessWithLists", @@ -73,7 +78,7 @@ function getDeclarationNameText(name) { return null; } -function containsStoreIdentifier(node) { +function containsPairingStoreSource(node) { let found = false; const visit = (current) => { if (found) { @@ -83,6 +88,13 @@ function containsStoreIdentifier(node) { found = true; return; } + if (ts.isCallExpression(current)) { + const callName = getCallName(current); + if (callName && storeSourceCallNames.has(callName)) { + found = true; + return; + } + } ts.forEachChild(current, visit); }; visit(node); @@ -123,7 +135,7 @@ function isSuspiciousNormalizeWithStoreCall(node) { if (!name) { continue; } - if (name === "storeAllowFrom" && containsStoreIdentifier(property.initializer)) { + if (name === "storeAllowFrom" && containsPairingStoreSource(property.initializer)) { hasStoreProp = true; } if (name === "allowFrom" && groupNameRe.test(property.initializer.getText())) { @@ -140,7 +152,7 @@ function findViolations(content, filePath) { const visit = (node) => { if (ts.isVariableDeclaration(node) && node.initializer) { const name = getDeclarationNameText(node.name); - if (name && groupNameRe.test(name) && containsStoreIdentifier(node.initializer)) { + if (name && groupNameRe.test(name) && containsPairingStoreSource(node.initializer)) { const callName = getCallName(node.initializer); if (callName && allowedResolverCallNames.has(callName)) { ts.forEachChild(node, visit); @@ -155,7 +167,7 @@ function findViolations(content, filePath) { if (ts.isPropertyAssignment(node)) { const propName = getPropertyNameText(node.name); - if (propName && groupNameRe.test(propName) && containsStoreIdentifier(node.initializer)) { + if (propName && groupNameRe.test(propName) && containsPairingStoreSource(node.initializer)) { violations.push({ line: toLine(sourceFile, node), reason: `group-scoped property "${propName}" references pairing-store identifiers`, diff --git a/src/discord/monitor/agent-components.ts b/src/discord/monitor/agent-components.ts index e39adf58165..bdfdbf5f167 100644 --- a/src/discord/monitor/agent-components.ts +++ b/src/discord/monitor/agent-components.ts @@ -41,6 +41,7 @@ import { } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { createNonExitingRuntime, type RuntimeEnv } from "../../runtime.js"; +import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js"; import { resolveDiscordComponentEntry, resolveDiscordModalEntry } from "../components-registry.js"; import { createDiscordFormModal, @@ -471,8 +472,11 @@ async function ensureDmComponentAuthorized(params: { return true; } - const storeAllowFrom = - dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("discord").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "discord", + dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const effectiveAllowFrom = [...(ctx.allowFrom ?? []), ...storeAllowFrom]; const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]); const allowMatch = allowList diff --git a/src/discord/monitor/listeners.ts b/src/discord/monitor/listeners.ts index c8af895ad25..b0aedf27593 100644 --- a/src/discord/monitor/listeners.ts +++ b/src/discord/monitor/listeners.ts @@ -13,7 +13,10 @@ import { enqueueSystemEvent } from "../../infra/system-events.js"; import { createSubsystemLogger } from "../../logging/subsystem.js"; import { readChannelAllowFromStore } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; -import { resolveDmGroupAccessWithLists } from "../../security/dm-policy-shared.js"; +import { + readStoreAllowFromForDmPolicy, + resolveDmGroupAccessWithLists, +} from "../../security/dm-policy-shared.js"; import { isDiscordGroupAllowedByPolicy, normalizeDiscordAllowList, @@ -233,10 +236,11 @@ async function authorizeDiscordReactionIngress( return { allowed: false, reason: "group-dm-disabled" }; } if (params.isDirectMessage) { - const storeAllowFrom = - params.dmPolicy === "allowlist" - ? [] - : await readChannelAllowFromStore("discord").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "discord", + dmPolicy: params.dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const access = resolveDmGroupAccessWithLists({ isGroup: false, dmPolicy: params.dmPolicy, diff --git a/src/discord/monitor/message-handler.preflight.ts b/src/discord/monitor/message-handler.preflight.ts index 8fa691ef078..27dff979c89 100644 --- a/src/discord/monitor/message-handler.preflight.ts +++ b/src/discord/monitor/message-handler.preflight.ts @@ -31,6 +31,7 @@ import { } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { resolveAgentIdFromSessionKey } from "../../routing/session-key.js"; +import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js"; import { fetchPluralKitMessageInfo } from "../pluralkit.js"; import { sendMessageDiscord } from "../send.js"; import { @@ -183,8 +184,11 @@ export async function preflightDiscordMessage( return null; } if (dmPolicy !== "open") { - const storeAllowFrom = - dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("discord").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "discord", + dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const effectiveAllowFrom = [...(params.allowFrom ?? []), ...storeAllowFrom]; const allowList = normalizeDiscordAllowList(effectiveAllowFrom, ["discord:", "user:", "pk:"]); const allowMatch = allowList diff --git a/src/discord/monitor/native-command.ts b/src/discord/monitor/native-command.ts index 1629f03fba1..24a6eb60147 100644 --- a/src/discord/monitor/native-command.ts +++ b/src/discord/monitor/native-command.ts @@ -53,6 +53,7 @@ import { import { resolveAgentRoute } from "../../routing/resolve-route.js"; import { resolveAgentIdFromSessionKey } from "../../routing/session-key.js"; import { buildUntrustedChannelMetadata } from "../../security/channel-metadata.js"; +import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js"; import { chunkItems } from "../../utils/chunk-items.js"; import { withTimeout } from "../../utils/with-timeout.js"; import { loadWebMedia } from "../../web/media.js"; @@ -1360,8 +1361,11 @@ async function dispatchDiscordCommandInteraction(params: { return; } if (dmPolicy !== "open") { - const storeAllowFrom = - dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("discord").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "discord", + dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const effectiveAllowFrom = [ ...(discordConfig?.allowFrom ?? discordConfig?.dm?.allowFrom ?? []), ...storeAllowFrom, diff --git a/src/imessage/monitor/inbound-processing.ts b/src/imessage/monitor/inbound-processing.ts index 4cfd1a4c99c..02917a3e5cb 100644 --- a/src/imessage/monitor/inbound-processing.ts +++ b/src/imessage/monitor/inbound-processing.ts @@ -20,7 +20,10 @@ import { resolveChannelGroupRequireMention, } from "../../config/group-policy.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; -import { resolveDmGroupAccessWithLists } from "../../security/dm-policy-shared.js"; +import { + DM_GROUP_ACCESS_REASON, + resolveDmGroupAccessWithLists, +} from "../../security/dm-policy-shared.js"; import { truncateUtf16Safe } from "../../utils.js"; import { formatIMessageChatTarget, @@ -162,24 +165,24 @@ export function resolveIMessageInboundDecision(params: { if (accessDecision.decision !== "allow") { if (isGroup) { - if (accessDecision.reason === "groupPolicy=disabled") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED) { params.logVerbose?.("Blocked iMessage group message (groupPolicy: disabled)"); return { kind: "drop", reason: "groupPolicy disabled" }; } - if (accessDecision.reason === "groupPolicy=allowlist (empty allowlist)") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST) { params.logVerbose?.( "Blocked iMessage group message (groupPolicy: allowlist, no groupAllowFrom)", ); return { kind: "drop", reason: "groupPolicy allowlist (empty groupAllowFrom)" }; } - if (accessDecision.reason === "groupPolicy=allowlist (not allowlisted)") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED) { params.logVerbose?.(`Blocked iMessage sender ${sender} (not in groupAllowFrom)`); return { kind: "drop", reason: "not in groupAllowFrom" }; } params.logVerbose?.(`Blocked iMessage group message (${accessDecision.reason})`); return { kind: "drop", reason: accessDecision.reason }; } - if (accessDecision.reason === "dmPolicy=disabled") { + if (accessDecision.reasonCode === DM_GROUP_ACCESS_REASON.DM_POLICY_DISABLED) { return { kind: "drop", reason: "dmPolicy disabled" }; } if (accessDecision.decision === "pairing") { diff --git a/src/plugin-sdk/index.ts b/src/plugin-sdk/index.ts index 0d378ec6c34..7036e71c2df 100644 --- a/src/plugin-sdk/index.ts +++ b/src/plugin-sdk/index.ts @@ -409,11 +409,14 @@ export { } from "../agents/tools/common.js"; export { formatDocsLink } from "../terminal/links.js"; export { + DM_GROUP_ACCESS_REASON, + readStoreAllowFromForDmPolicy, resolveDmAllowState, resolveDmGroupAccessDecision, resolveDmGroupAccessWithLists, resolveEffectiveAllowFromLists, } from "../security/dm-policy-shared.js"; +export type { DmGroupAccessReasonCode } from "../security/dm-policy-shared.js"; export type { HookEntry } from "../hooks/types.js"; export { clamp, escapeRegExp, normalizeE164, safeParseJson, sleep } from "../utils.js"; export { stripAnsi } from "../terminal/ansi.js"; diff --git a/src/security/dm-policy-channel-smoke.test.ts b/src/security/dm-policy-channel-smoke.test.ts index 05cd67c47de..7a57317d628 100644 --- a/src/security/dm-policy-channel-smoke.test.ts +++ b/src/security/dm-policy-channel-smoke.test.ts @@ -2,7 +2,7 @@ import { describe, expect, it } from "vitest"; import { isAllowedBlueBubblesSender } from "../../extensions/bluebubbles/src/targets.js"; import { isMattermostSenderAllowed } from "../../extensions/mattermost/src/mattermost/monitor-auth.js"; import { isSignalSenderAllowed, type SignalSender } from "../signal/identity.js"; -import { resolveDmGroupAccessWithLists } from "./dm-policy-shared.js"; +import { DM_GROUP_ACCESS_REASON, resolveDmGroupAccessWithLists } from "./dm-policy-shared.js"; type ChannelSmokeCase = { name: string; @@ -58,6 +58,7 @@ describe("security/dm-policy-shared channel smoke", () => { isSenderAllowed: testCase.isSenderAllowed, }); expect(access.decision).toBe("block"); + expect(access.reasonCode).toBe(DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED); expect(access.reason).toBe("groupPolicy=allowlist (not allowlisted)"); }); } diff --git a/src/security/dm-policy-shared.test.ts b/src/security/dm-policy-shared.test.ts index 4e5f0dbf832..2be2e9d46b5 100644 --- a/src/security/dm-policy-shared.test.ts +++ b/src/security/dm-policy-shared.test.ts @@ -1,5 +1,7 @@ import { describe, expect, it } from "vitest"; import { + DM_GROUP_ACCESS_REASON, + readStoreAllowFromForDmPolicy, resolveDmAllowState, resolveDmGroupAccessDecision, resolveDmGroupAccessWithLists, @@ -34,6 +36,34 @@ describe("security/dm-policy-shared", () => { expect(state.isMultiUserDm).toBe(false); }); + it("skips pairing-store reads when dmPolicy is allowlist", async () => { + let called = false; + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "telegram", + dmPolicy: "allowlist", + readStore: async () => { + called = true; + return ["should-not-be-read"]; + }, + }); + expect(called).toBe(false); + expect(storeAllowFrom).toEqual([]); + }); + + it("skips pairing-store reads when shouldRead=false", async () => { + let called = false; + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "slack", + shouldRead: false, + readStore: async () => { + called = true; + return ["should-not-be-read"]; + }, + }); + expect(called).toBe(false); + expect(storeAllowFrom).toEqual([]); + }); + it("builds effective DM/group allowlists from config + pairing store", () => { const lists = resolveEffectiveAllowFromLists({ allowFrom: [" owner ", "", "owner2"], @@ -98,6 +128,7 @@ describe("security/dm-policy-shared", () => { isSenderAllowed: (allowFrom) => allowFrom.includes("paired-user"), }); expect(resolved.decision).toBe("allow"); + expect(resolved.reasonCode).toBe(DM_GROUP_ACCESS_REASON.DM_POLICY_ALLOWLISTED); expect(resolved.reason).toBe("dmPolicy=pairing (allowlisted)"); expect(resolved.effectiveAllowFrom).toEqual(["owner", "paired-user"]); expect(resolved.effectiveGroupAllowFrom).toEqual(["group:room"]); @@ -114,6 +145,7 @@ describe("security/dm-policy-shared", () => { isSenderAllowed: () => false, }); expect(resolved.decision).toBe("block"); + expect(resolved.reasonCode).toBe(DM_GROUP_ACCESS_REASON.DM_POLICY_NOT_ALLOWLISTED); expect(resolved.reason).toBe("dmPolicy=allowlist (not allowlisted)"); expect(resolved.effectiveAllowFrom).toEqual(["owner"]); }); @@ -237,6 +269,7 @@ describe("security/dm-policy-shared", () => { }); expect(decision).toEqual({ decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_NOT_ALLOWLISTED, reason: "dmPolicy=allowlist (not allowlisted)", }); }); @@ -252,6 +285,7 @@ describe("security/dm-policy-shared", () => { }); expect(decision).toEqual({ decision: "pairing", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_PAIRING_REQUIRED, reason: "dmPolicy=pairing (not allowlisted)", }); }); @@ -279,6 +313,7 @@ describe("security/dm-policy-shared", () => { }); expect(decision).toEqual({ decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED, reason: "groupPolicy=allowlist (not allowlisted)", }); }); diff --git a/src/security/dm-policy-shared.ts b/src/security/dm-policy-shared.ts index 89f7740ce05..e5a80451868 100644 --- a/src/security/dm-policy-shared.ts +++ b/src/security/dm-policy-shared.ts @@ -35,6 +35,31 @@ export function resolveEffectiveAllowFromLists(params: { } export type DmGroupAccessDecision = "allow" | "block" | "pairing"; +export const DM_GROUP_ACCESS_REASON = { + GROUP_POLICY_ALLOWED: "group_policy_allowed", + GROUP_POLICY_DISABLED: "group_policy_disabled", + GROUP_POLICY_EMPTY_ALLOWLIST: "group_policy_empty_allowlist", + GROUP_POLICY_NOT_ALLOWLISTED: "group_policy_not_allowlisted", + DM_POLICY_OPEN: "dm_policy_open", + DM_POLICY_DISABLED: "dm_policy_disabled", + DM_POLICY_ALLOWLISTED: "dm_policy_allowlisted", + DM_POLICY_PAIRING_REQUIRED: "dm_policy_pairing_required", + DM_POLICY_NOT_ALLOWLISTED: "dm_policy_not_allowlisted", +} as const; +export type DmGroupAccessReasonCode = + (typeof DM_GROUP_ACCESS_REASON)[keyof typeof DM_GROUP_ACCESS_REASON]; + +export async function readStoreAllowFromForDmPolicy(params: { + provider: ChannelId; + dmPolicy?: string | null; + shouldRead?: boolean | null; + readStore?: (provider: ChannelId) => Promise; +}): Promise { + if (params.shouldRead === false || params.dmPolicy === "allowlist") { + return []; + } + return await (params.readStore ?? readChannelAllowFromStore)(params.provider).catch(() => []); +} export function resolveDmGroupAccessDecision(params: { isGroup: boolean; @@ -45,6 +70,7 @@ export function resolveDmGroupAccessDecision(params: { isSenderAllowed: (allowFrom: string[]) => boolean; }): { decision: DmGroupAccessDecision; + reasonCode: DmGroupAccessReasonCode; reason: string; } { const dmPolicy = params.dmPolicy ?? "pairing"; @@ -54,32 +80,68 @@ export function resolveDmGroupAccessDecision(params: { if (params.isGroup) { if (groupPolicy === "disabled") { - return { decision: "block", reason: "groupPolicy=disabled" }; + return { + decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED, + reason: "groupPolicy=disabled", + }; } if (groupPolicy === "allowlist") { if (effectiveGroupAllowFrom.length === 0) { - return { decision: "block", reason: "groupPolicy=allowlist (empty allowlist)" }; + return { + decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST, + reason: "groupPolicy=allowlist (empty allowlist)", + }; } if (!params.isSenderAllowed(effectiveGroupAllowFrom)) { - return { decision: "block", reason: "groupPolicy=allowlist (not allowlisted)" }; + return { + decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_NOT_ALLOWLISTED, + reason: "groupPolicy=allowlist (not allowlisted)", + }; } } - return { decision: "allow", reason: `groupPolicy=${groupPolicy}` }; + return { + decision: "allow", + reasonCode: DM_GROUP_ACCESS_REASON.GROUP_POLICY_ALLOWED, + reason: `groupPolicy=${groupPolicy}`, + }; } if (dmPolicy === "disabled") { - return { decision: "block", reason: "dmPolicy=disabled" }; + return { + decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_DISABLED, + reason: "dmPolicy=disabled", + }; } if (dmPolicy === "open") { - return { decision: "allow", reason: "dmPolicy=open" }; + return { + decision: "allow", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_OPEN, + reason: "dmPolicy=open", + }; } if (params.isSenderAllowed(effectiveAllowFrom)) { - return { decision: "allow", reason: `dmPolicy=${dmPolicy} (allowlisted)` }; + return { + decision: "allow", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_ALLOWLISTED, + reason: `dmPolicy=${dmPolicy} (allowlisted)`, + }; } if (dmPolicy === "pairing") { - return { decision: "pairing", reason: "dmPolicy=pairing (not allowlisted)" }; + return { + decision: "pairing", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_PAIRING_REQUIRED, + reason: "dmPolicy=pairing (not allowlisted)", + }; } - return { decision: "block", reason: `dmPolicy=${dmPolicy} (not allowlisted)` }; + return { + decision: "block", + reasonCode: DM_GROUP_ACCESS_REASON.DM_POLICY_NOT_ALLOWLISTED, + reason: `dmPolicy=${dmPolicy} (not allowlisted)`, + }; } export function resolveDmGroupAccessWithLists(params: { @@ -93,6 +155,7 @@ export function resolveDmGroupAccessWithLists(params: { isSenderAllowed: (allowFrom: string[]) => boolean; }): { decision: DmGroupAccessDecision; + reasonCode: DmGroupAccessReasonCode; reason: string; effectiveAllowFrom: string[]; effectiveGroupAllowFrom: string[]; @@ -134,9 +197,10 @@ export async function resolveDmAllowState(params: { Array.isArray(params.allowFrom) ? params.allowFrom : undefined, ); const hasWildcard = configAllowFrom.includes("*"); - const storeAllowFrom = await (params.readStore ?? readChannelAllowFromStore)( - params.provider, - ).catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: params.provider, + readStore: params.readStore, + }); const normalizeEntry = params.normalizeEntry ?? ((value: string) => value); const normalizedCfg = configAllowFrom .filter((value) => value !== "*") diff --git a/src/signal/monitor/event-handler.ts b/src/signal/monitor/event-handler.ts index ea9fa9f49d6..c275d090e71 100644 --- a/src/signal/monitor/event-handler.ts +++ b/src/signal/monitor/event-handler.ts @@ -36,7 +36,11 @@ import { upsertChannelPairingRequest, } from "../../pairing/pairing-store.js"; import { resolveAgentRoute } from "../../routing/resolve-route.js"; -import { resolveDmGroupAccessWithLists } from "../../security/dm-policy-shared.js"; +import { + DM_GROUP_ACCESS_REASON, + readStoreAllowFromForDmPolicy, + resolveDmGroupAccessWithLists, +} from "../../security/dm-policy-shared.js"; import { normalizeE164 } from "../../utils.js"; import { formatSignalPairingIdLine, @@ -453,10 +457,11 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { const hasBodyContent = Boolean(messageText || quoteText) || Boolean(!reaction && dataMessage?.attachments?.length); const senderDisplay = formatSignalSenderDisplay(sender); - const storeAllowFrom = - deps.dmPolicy === "allowlist" - ? [] - : await readChannelAllowFromStore("signal").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "signal", + dmPolicy: deps.dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const resolveAccessDecision = (isGroup: boolean) => resolveDmGroupAccessWithLists({ isGroup, @@ -543,9 +548,9 @@ export function createSignalEventHandler(deps: SignalEventHandlerDeps) { if (isGroup) { const groupAccess = resolveAccessDecision(true); if (groupAccess.decision !== "allow") { - if (groupAccess.reason === "groupPolicy=disabled") { + if (groupAccess.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_DISABLED) { logVerbose("Blocked signal group message (groupPolicy: disabled)"); - } else if (groupAccess.reason === "groupPolicy=allowlist (empty allowlist)") { + } else if (groupAccess.reasonCode === DM_GROUP_ACCESS_REASON.GROUP_POLICY_EMPTY_ALLOWLIST) { logVerbose("Blocked signal group message (groupPolicy: allowlist, no groupAllowFrom)"); } else { logVerbose(`Blocked signal group sender ${senderDisplay} (not in groupAllowFrom)`); diff --git a/src/slack/monitor/auth.ts b/src/slack/monitor/auth.ts index cb43241f899..9521ca3c007 100644 --- a/src/slack/monitor/auth.ts +++ b/src/slack/monitor/auth.ts @@ -1,4 +1,5 @@ import { readChannelAllowFromStore } from "../../pairing/pairing-store.js"; +import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js"; import { allowListMatches, normalizeAllowList, @@ -9,8 +10,11 @@ import { resolveSlackChannelConfig } from "./channel-config.js"; import { normalizeSlackChannelType, type SlackMonitorContext } from "./context.js"; export async function resolveSlackEffectiveAllowFrom(ctx: SlackMonitorContext) { - const storeAllowFrom = - ctx.dmPolicy === "allowlist" ? [] : await readChannelAllowFromStore("slack").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "slack", + dmPolicy: ctx.dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const allowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]); const allowFromLower = normalizeAllowListLower(allowFrom); return { allowFrom, allowFromLower }; diff --git a/src/slack/monitor/slash.ts b/src/slack/monitor/slash.ts index 92afc734a91..e65fbf62c41 100644 --- a/src/slack/monitor/slash.ts +++ b/src/slack/monitor/slash.ts @@ -10,6 +10,7 @@ import { readChannelAllowFromStore, upsertChannelPairingRequest, } from "../../pairing/pairing-store.js"; +import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js"; import { chunkItems } from "../../utils/chunk-items.js"; import type { ResolvedSlackAccount } from "../accounts.js"; import { @@ -335,10 +336,11 @@ export async function registerSlackMonitorSlashCommands(params: { return; } - const storeAllowFrom = - ctx.dmPolicy === "allowlist" - ? [] - : await readChannelAllowFromStore("slack").catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "slack", + dmPolicy: ctx.dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider), + }); const effectiveAllowFrom = normalizeAllowList([...ctx.allowFrom, ...storeAllowFrom]); const effectiveAllowFromLower = normalizeAllowListLower(effectiveAllowFrom); diff --git a/src/web/auto-reply/monitor/process-message.ts b/src/web/auto-reply/monitor/process-message.ts index 3ef85b6eb2d..56ca1b7aa8b 100644 --- a/src/web/auto-reply/monitor/process-message.ts +++ b/src/web/auto-reply/monitor/process-message.ts @@ -27,6 +27,7 @@ import type { getChildLogger } from "../../../logging.js"; import { getAgentScopedMediaLocalRoots } from "../../../media/local-roots.js"; import { readChannelAllowFromStore } from "../../../pairing/pairing-store.js"; import type { resolveAgentRoute } from "../../../routing/resolve-route.js"; +import { readStoreAllowFromForDmPolicy } from "../../../security/dm-policy-shared.js"; import { jidToE164, normalizeE164 } from "../../../utils.js"; import { resolveWhatsAppAccount } from "../../accounts.js"; import { newConnectionId } from "../../reconnect.js"; @@ -90,12 +91,11 @@ async function resolveWhatsAppCommandAuthorized(params: { return normalizeAllowFromE164(configuredGroupAllowFrom).includes(senderE164); } - const storeAllowFrom = - dmPolicy === "allowlist" - ? [] - : await readChannelAllowFromStore("whatsapp", process.env, params.msg.accountId).catch( - () => [], - ); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "whatsapp", + dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider, process.env, params.msg.accountId), + }); const combinedAllowFrom = Array.from( new Set([...(configuredAllowFrom ?? []), ...storeAllowFrom]), ); diff --git a/src/web/inbound/access-control.ts b/src/web/inbound/access-control.ts index 2e759507cb9..439bc534d62 100644 --- a/src/web/inbound/access-control.ts +++ b/src/web/inbound/access-control.ts @@ -10,6 +10,7 @@ import { readChannelAllowFromStore, upsertChannelPairingRequest, } from "../../pairing/pairing-store.js"; +import { readStoreAllowFromForDmPolicy } from "../../security/dm-policy-shared.js"; import { isSelfChatMode, normalizeE164 } from "../../utils.js"; import { resolveWhatsAppAccount } from "../accounts.js"; @@ -60,10 +61,11 @@ export async function checkInboundAccessControl(params: { }); const dmPolicy = account.dmPolicy ?? "pairing"; const configuredAllowFrom = account.allowFrom; - const storeAllowFrom = - dmPolicy === "allowlist" - ? [] - : await readChannelAllowFromStore("whatsapp", process.env, account.accountId).catch(() => []); + const storeAllowFrom = await readStoreAllowFromForDmPolicy({ + provider: "whatsapp", + dmPolicy, + readStore: (provider) => readChannelAllowFromStore(provider, process.env, account.accountId), + }); // Without user config, default to self-only DM access so the owner can talk to themselves. const combinedAllowFrom = Array.from( new Set([...(configuredAllowFrom ?? []), ...storeAllowFrom]),