diff --git a/src/auto-reply/command-auth.owner-allow-from-cache.test.ts b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts new file mode 100644 index 00000000000..23023d7aa35 --- /dev/null +++ b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts @@ -0,0 +1,165 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import type { OpenClawConfig } from "../config/config.js"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import { createOutboundTestPlugin, createTestRegistry } from "../test-utils/channel-plugins.js"; +import { resolveCommandAuthorization } from "./command-auth.js"; +import type { MsgContext } from "./templating.js"; + +const largeListFormatter = vi.fn(); + +describe("resolveCommandAuthorization owner allowlist hot path", () => { + beforeEach(() => { + largeListFormatter.mockReset(); + const plugin = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + plugin.config = { + ...plugin.config, + formatAllowFrom: ({ allowFrom }) => { + if (allowFrom.length > 1) { + largeListFormatter(); + } + return allowFrom + .map((entry) => String(entry ?? "").trim()) + .filter((entry) => entry.length > 0); + }, + }; + setActivePluginRegistry(createTestRegistry([{ pluginId: "discord", plugin, source: "test" }])); + }); + + afterEach(() => { + setActivePluginRegistry(createTestRegistry()); + }); + + it("reuses processed ownerAllowFrom for repeated authorizations on the same config snapshot", () => { + const cfg = { + channels: { discord: {} }, + commands: { ownerAllowFrom: ["123", "456", "789"] }, + } as OpenClawConfig; + + const ctx = { + Provider: "discord", + Surface: "discord", + From: "discord:123", + SenderId: "123", + } as MsgContext; + + resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + expect(largeListFormatter).toHaveBeenCalledTimes(1); + }); + + it("does not reuse cached ownerAllowFrom across config snapshots sharing the same array", () => { + const sharedOwnerAllowFrom = ["123", "456", "789"]; + const cfgA = { + channels: { discord: {} }, + commands: { ownerAllowFrom: sharedOwnerAllowFrom }, + testVariant: "A", + } as OpenClawConfig; + const cfgB = { + channels: { discord: {} }, + commands: { ownerAllowFrom: sharedOwnerAllowFrom }, + testVariant: "B", + } as OpenClawConfig; + + const plugin = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + plugin.config = { + ...plugin.config, + formatAllowFrom: ({ cfg, allowFrom }) => + allowFrom.map( + (entry) => `${(cfg as { testVariant?: string }).testVariant}:${String(entry)}`, + ), + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin, source: "test" }]), + "owner-cache-config-a", + ); + + const ctx = { + Provider: "discord", + Surface: "discord", + From: "discord:123", + SenderId: "123", + } as MsgContext; + + const authA = resolveCommandAuthorization({ + ctx, + cfg: cfgA, + commandAuthorized: true, + }); + const authB = resolveCommandAuthorization({ + ctx, + cfg: cfgB, + commandAuthorized: true, + }); + + expect(authA.ownerList).toEqual(["A:123", "A:456", "A:789"]); + expect(authB.ownerList).toEqual(["B:123", "B:456", "B:789"]); + }); + + it("does not reuse cached ownerAllowFrom across plugin registry reloads", () => { + const sharedOwnerAllowFrom = ["123", "456", "789"]; + const cfg = { + channels: { discord: {} }, + commands: { ownerAllowFrom: sharedOwnerAllowFrom }, + } as OpenClawConfig; + const ctx = { + Provider: "discord", + Surface: "discord", + From: "discord:123", + SenderId: "123", + } as MsgContext; + + const pluginA = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + pluginA.config = { + ...pluginA.config, + formatAllowFrom: ({ allowFrom }) => allowFrom.map((entry) => `A:${String(entry)}`), + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin: pluginA, source: "test" }]), + "owner-cache-registry-a", + ); + const authA = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + const pluginB = createOutboundTestPlugin({ + id: "discord", + outbound: { deliveryMode: "direct" }, + }); + pluginB.config = { + ...pluginB.config, + formatAllowFrom: ({ allowFrom }) => allowFrom.map((entry) => `B:${String(entry)}`), + }; + setActivePluginRegistry( + createTestRegistry([{ pluginId: "discord", plugin: pluginB, source: "test" }]), + "owner-cache-registry-b", + ); + const authB = resolveCommandAuthorization({ + ctx, + cfg, + commandAuthorized: true, + }); + + expect(authA.ownerList).toEqual(["A:123", "A:456", "A:789"]); + expect(authB.ownerList).toEqual(["B:123", "B:456", "B:789"]); + }); +}); diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 44e9f5f30db..75f9d95140c 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -2,6 +2,7 @@ import { getChannelPlugin, listChannelPlugins } from "../channels/plugins/index. import type { ChannelId, ChannelPlugin } from "../channels/plugins/types.js"; import { normalizeAnyChannelId } from "../channels/registry.js"; import type { OpenClawConfig } from "../config/config.js"; +import { getActivePluginRegistryVersion } from "../plugins/runtime.js"; import { normalizeStringEntries } from "../shared/string-normalization.js"; import { INTERNAL_MESSAGE_CHANNEL, @@ -20,6 +21,11 @@ export type CommandAuthorization = { to?: string; }; +const ownerAllowFromListCache = new WeakMap< + OpenClawConfig, + WeakMap, Map> +>(); + function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined { const explicitMessageChannel = normalizeMessageChannel(ctx.Provider) ?? @@ -116,6 +122,12 @@ function resolveOwnerAllowFromList(params: { if (!Array.isArray(raw) || raw.length === 0) { return []; } + const registryVersion = getActivePluginRegistryVersion(); + const cacheKey = `${registryVersion}\u0000${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`; + const cached = ownerAllowFromListCache.get(params.cfg)?.get(raw)?.get(cacheKey); + if (cached !== undefined) { + return cached; + } const filtered: string[] = []; for (const entry of raw) { const trimmed = String(entry ?? "").trim(); @@ -139,12 +151,24 @@ function resolveOwnerAllowFromList(params: { } filtered.push(trimmed); } - return formatAllowFromList({ + const formatted = formatAllowFromList({ plugin: params.plugin, cfg: params.cfg, accountId: params.accountId, allowFrom: filtered, }); + let cachedByConfig = ownerAllowFromListCache.get(params.cfg); + if (!cachedByConfig) { + cachedByConfig = new WeakMap, Map>(); + ownerAllowFromListCache.set(params.cfg, cachedByConfig); + } + let cachedByKey = cachedByConfig.get(raw); + if (!cachedByKey) { + cachedByKey = new Map(); + cachedByConfig.set(raw, cachedByKey); + } + cachedByKey.set(cacheKey, formatted); + return formatted; } /** @@ -244,16 +268,14 @@ function resolveSenderCandidates(params: { pushCandidate(params.from); } - const normalized: string[] = []; + const normalized = new Set(); for (const sender of candidates) { const entries = normalizeAllowFromEntry({ plugin, cfg, accountId, value: sender }); for (const entry of entries) { - if (!normalized.includes(entry)) { - normalized.push(entry); - } + normalized.add(entry); } } - return normalized; + return [...normalized]; } function resolveFallbackAllowFrom(params: { @@ -339,13 +361,16 @@ export function resolveCommandAuthorization(params: { providerId, allowFrom: cfg.commands?.ownerAllowFrom, }); - const contextOwnerAllowFromList = resolveOwnerAllowFromList({ - plugin, - cfg, - accountId: ctx.AccountId, - providerId, - allowFrom: ctx.OwnerAllowFrom, - }); + const contextOwnerAllowFromList = + Array.isArray(ctx.OwnerAllowFrom) && ctx.OwnerAllowFrom.length > 0 + ? resolveOwnerAllowFromList({ + plugin, + cfg, + accountId: ctx.AccountId, + providerId, + allowFrom: ctx.OwnerAllowFrom, + }) + : []; const allowAll = allowFromList.length === 0 || allowFromList.some((entry) => entry.trim() === "*"); @@ -375,6 +400,8 @@ export function resolveCommandAuthorization(params: { : ownerCandidatesForCommands, ), ); + const ownerListSet = new Set(ownerList); + const ownerCandidatesForCommandsSet = new Set(ownerCandidatesForCommands); const senderCandidates = resolveSenderCandidates({ plugin, @@ -386,11 +413,11 @@ export function resolveCommandAuthorization(params: { from, chatType: ctx.ChatType, }); - const matchedSender = ownerList.length - ? senderCandidates.find((candidate) => ownerList.includes(candidate)) + const matchedSender = ownerListSet.size + ? senderCandidates.find((candidate) => ownerListSet.has(candidate)) : undefined; - const matchedCommandOwner = ownerCandidatesForCommands.length - ? senderCandidates.find((candidate) => ownerCandidatesForCommands.includes(candidate)) + const matchedCommandOwner = ownerCandidatesForCommandsSet.size + ? senderCandidates.find((candidate) => ownerCandidatesForCommandsSet.has(candidate)) : undefined; const senderId = matchedSender ?? senderCandidates[0]; @@ -420,8 +447,9 @@ export function resolveCommandAuthorization(params: { if (commandsAllowFromList !== null) { // commands.allowFrom is configured - use it for authorization const commandsAllowAll = commandsAllowFromList.some((entry) => entry.trim() === "*"); - const matchedCommandsAllowFrom = commandsAllowFromList.length - ? senderCandidates.find((candidate) => commandsAllowFromList.includes(candidate)) + const commandsAllowFromSet = new Set(commandsAllowFromList); + const matchedCommandsAllowFrom = commandsAllowFromSet.size + ? senderCandidates.find((candidate) => commandsAllowFromSet.has(candidate)) : undefined; isAuthorizedSender = commandsAllowAll || Boolean(matchedCommandsAllowFrom); } else {