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..4cd25adb8a3 --- /dev/null +++ b/src/auto-reply/command-auth.owner-allow-from-cache.test.ts @@ -0,0 +1,63 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { setActivePluginRegistry } from "../plugins/runtime.js"; +import type { OpenClawConfig } from "../config/config.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); + }); +}); diff --git a/src/auto-reply/command-auth.ts b/src/auto-reply/command-auth.ts index 44e9f5f30db..1ccec930e96 100644 --- a/src/auto-reply/command-auth.ts +++ b/src/auto-reply/command-auth.ts @@ -20,6 +20,8 @@ export type CommandAuthorization = { to?: string; }; +const ownerAllowFromListCache = new WeakMap, Map>(); + function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined { const explicitMessageChannel = normalizeMessageChannel(ctx.Provider) ?? @@ -116,6 +118,11 @@ function resolveOwnerAllowFromList(params: { if (!Array.isArray(raw) || raw.length === 0) { return []; } + const cacheKey = `${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`; + const cached = ownerAllowFromListCache.get(raw)?.get(cacheKey); + if (cached) { + return cached; + } const filtered: string[] = []; for (const entry of raw) { const trimmed = String(entry ?? "").trim(); @@ -139,12 +146,19 @@ function resolveOwnerAllowFromList(params: { } filtered.push(trimmed); } - return formatAllowFromList({ + const formatted = formatAllowFromList({ plugin: params.plugin, cfg: params.cfg, accountId: params.accountId, allowFrom: filtered, }); + let cachedByKey = ownerAllowFromListCache.get(raw); + if (!cachedByKey) { + cachedByKey = new Map(); + ownerAllowFromListCache.set(raw, cachedByKey); + } + cachedByKey.set(cacheKey, formatted); + return formatted; } /** @@ -244,16 +258,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 +351,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 +390,8 @@ export function resolveCommandAuthorization(params: { : ownerCandidatesForCommands, ), ); + const ownerListSet = new Set(ownerList); + const ownerCandidatesForCommandsSet = new Set(ownerCandidatesForCommands); const senderCandidates = resolveSenderCandidates({ plugin, @@ -386,11 +403,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 +437,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 {