fix(commands): scope owner allowlist cache to config and registry
This commit is contained in:
parent
7bd2f391b0
commit
10da4efff9
@ -60,4 +60,106 @@ describe("resolveCommandAuthorization owner allowlist hot path", () => {
|
|||||||
|
|
||||||
expect(largeListFormatter).toHaveBeenCalledTimes(1);
|
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"]);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@ -2,6 +2,7 @@ import { getChannelPlugin, listChannelPlugins } from "../channels/plugins/index.
|
|||||||
import type { ChannelId, ChannelPlugin } from "../channels/plugins/types.js";
|
import type { ChannelId, ChannelPlugin } from "../channels/plugins/types.js";
|
||||||
import { normalizeAnyChannelId } from "../channels/registry.js";
|
import { normalizeAnyChannelId } from "../channels/registry.js";
|
||||||
import type { OpenClawConfig } from "../config/config.js";
|
import type { OpenClawConfig } from "../config/config.js";
|
||||||
|
import { getActivePluginRegistryVersion } from "../plugins/runtime.js";
|
||||||
import { normalizeStringEntries } from "../shared/string-normalization.js";
|
import { normalizeStringEntries } from "../shared/string-normalization.js";
|
||||||
import {
|
import {
|
||||||
INTERNAL_MESSAGE_CHANNEL,
|
INTERNAL_MESSAGE_CHANNEL,
|
||||||
@ -20,7 +21,10 @@ export type CommandAuthorization = {
|
|||||||
to?: string;
|
to?: string;
|
||||||
};
|
};
|
||||||
|
|
||||||
const ownerAllowFromListCache = new WeakMap<ReadonlyArray<string | number>, Map<string, string[]>>();
|
const ownerAllowFromListCache = new WeakMap<
|
||||||
|
OpenClawConfig,
|
||||||
|
WeakMap<ReadonlyArray<string | number>, Map<string, string[]>>
|
||||||
|
>();
|
||||||
|
|
||||||
function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined {
|
function resolveProviderFromContext(ctx: MsgContext, cfg: OpenClawConfig): ChannelId | undefined {
|
||||||
const explicitMessageChannel =
|
const explicitMessageChannel =
|
||||||
@ -118,9 +122,10 @@ function resolveOwnerAllowFromList(params: {
|
|||||||
if (!Array.isArray(raw) || raw.length === 0) {
|
if (!Array.isArray(raw) || raw.length === 0) {
|
||||||
return [];
|
return [];
|
||||||
}
|
}
|
||||||
const cacheKey = `${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`;
|
const registryVersion = getActivePluginRegistryVersion();
|
||||||
const cached = ownerAllowFromListCache.get(raw)?.get(cacheKey);
|
const cacheKey = `${registryVersion}\u0000${params.plugin?.id ?? ""}\u0000${params.accountId ?? ""}\u0000${params.providerId ?? ""}`;
|
||||||
if (cached) {
|
const cached = ownerAllowFromListCache.get(params.cfg)?.get(raw)?.get(cacheKey);
|
||||||
|
if (cached !== undefined) {
|
||||||
return cached;
|
return cached;
|
||||||
}
|
}
|
||||||
const filtered: string[] = [];
|
const filtered: string[] = [];
|
||||||
@ -152,10 +157,15 @@ function resolveOwnerAllowFromList(params: {
|
|||||||
accountId: params.accountId,
|
accountId: params.accountId,
|
||||||
allowFrom: filtered,
|
allowFrom: filtered,
|
||||||
});
|
});
|
||||||
let cachedByKey = ownerAllowFromListCache.get(raw);
|
let cachedByConfig = ownerAllowFromListCache.get(params.cfg);
|
||||||
|
if (!cachedByConfig) {
|
||||||
|
cachedByConfig = new WeakMap<ReadonlyArray<string | number>, Map<string, string[]>>();
|
||||||
|
ownerAllowFromListCache.set(params.cfg, cachedByConfig);
|
||||||
|
}
|
||||||
|
let cachedByKey = cachedByConfig.get(raw);
|
||||||
if (!cachedByKey) {
|
if (!cachedByKey) {
|
||||||
cachedByKey = new Map<string, string[]>();
|
cachedByKey = new Map<string, string[]>();
|
||||||
ownerAllowFromListCache.set(raw, cachedByKey);
|
cachedByConfig.set(raw, cachedByKey);
|
||||||
}
|
}
|
||||||
cachedByKey.set(cacheKey, formatted);
|
cachedByKey.set(cacheKey, formatted);
|
||||||
return formatted;
|
return formatted;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user