From b2589ac4518ee75e95025163a5b50cb8cc97d0f6 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 13 Apr 2026 17:47:05 +0100 Subject: [PATCH] perf(cron): use read-only allow-from store seam --- .../isolated-agent/delivery-target.test.ts | 10 +- src/cron/isolated-agent/delivery-target.ts | 8 +- src/pairing/allow-from-store-read.test.ts | 89 +++++ src/pairing/allow-from-store-read.ts | 322 ++++++++++++++++++ 4 files changed, 421 insertions(+), 8 deletions(-) create mode 100644 src/pairing/allow-from-store-read.test.ts create mode 100644 src/pairing/allow-from-store-read.ts diff --git a/src/cron/isolated-agent/delivery-target.test.ts b/src/cron/isolated-agent/delivery-target.test.ts index 5e0541b0f3a..d60eadcc487 100644 --- a/src/cron/isolated-agent/delivery-target.test.ts +++ b/src/cron/isolated-agent/delivery-target.test.ts @@ -27,8 +27,8 @@ vi.mock("../../infra/outbound/target-id-resolution.js", () => ({ maybeResolveIdLikeTarget: vi.fn(), })); -vi.mock("../../pairing/pairing-store.js", () => ({ - readChannelAllowFromStoreSync: vi.fn(() => []), +vi.mock("../../pairing/allow-from-store-read.js", () => ({ + readChannelAllowFromStoreEntriesSync: vi.fn(() => []), })); vi.mock("../../infra/outbound/targets.runtime.js", () => ({ @@ -41,14 +41,14 @@ const mockedModuleIds = [ "../../infra/outbound/channel-selection.runtime.js", "../../infra/outbound/targets.runtime.js", "../../infra/outbound/target-id-resolution.js", - "../../pairing/pairing-store.js", + "../../pairing/allow-from-store-read.js", ]; import { loadSessionStore } from "../../config/sessions/store-load.js"; import { resolveMessageChannelSelection } from "../../infra/outbound/channel-selection.runtime.js"; import { maybeResolveIdLikeTarget } from "../../infra/outbound/target-id-resolution.js"; import { resolveOutboundTarget } from "../../infra/outbound/targets.runtime.js"; -import { readChannelAllowFromStoreSync } from "../../pairing/pairing-store.js"; +import { readChannelAllowFromStoreEntriesSync } from "../../pairing/allow-from-store-read.js"; import { resolveDeliveryTarget } from "./delivery-target.js"; afterAll(() => { @@ -178,7 +178,7 @@ function setLastSessionEntry(params: { } function setStoredWhatsAppAllowFrom(allowFrom: string[]) { - vi.mocked(readChannelAllowFromStoreSync).mockReturnValue(allowFrom); + vi.mocked(readChannelAllowFromStoreEntriesSync).mockReturnValue(allowFrom); } async function resolveForAgent(params: { diff --git a/src/cron/isolated-agent/delivery-target.ts b/src/cron/isolated-agent/delivery-target.ts index 183130381ea..b97b1afcef4 100644 --- a/src/cron/isolated-agent/delivery-target.ts +++ b/src/cron/isolated-agent/delivery-target.ts @@ -9,7 +9,7 @@ import { maybeResolveIdLikeTarget } from "../../infra/outbound/target-id-resolut import { tryResolveLoadedOutboundTarget } from "../../infra/outbound/targets-loaded.js"; import { resolveSessionDeliveryTarget } from "../../infra/outbound/targets-session.js"; import type { OutboundChannel } from "../../infra/outbound/targets.js"; -import { readChannelAllowFromStoreSync } from "../../pairing/pairing-store.js"; +import { readChannelAllowFromStoreEntriesSync } from "../../pairing/allow-from-store-read.js"; import { mapAllowFromEntries } from "../../plugin-sdk/channel-config-helpers.js"; import { buildChannelAccountBindings } from "../../routing/bindings.js"; import { normalizeAccountId, normalizeAgentId } from "../../routing/session-key.js"; @@ -186,8 +186,10 @@ export async function resolveDeliveryTarget( const configuredAllowFrom = configuredAllowFromRaw ? mapAllowFromEntries(configuredAllowFromRaw) : []; - const storeAllowFrom = mapAllowFromEntries( - readChannelAllowFromStoreSync(channel, process.env, resolvedAccountId), + const storeAllowFrom = readChannelAllowFromStoreEntriesSync( + channel, + process.env, + resolvedAccountId, ); const allowFromOverride = [...new Set([...configuredAllowFrom, ...storeAllowFrom])]; const effectiveAllowFrom = mode === "implicit" ? allowFromOverride : undefined; diff --git a/src/pairing/allow-from-store-read.test.ts b/src/pairing/allow-from-store-read.test.ts new file mode 100644 index 00000000000..8a936345820 --- /dev/null +++ b/src/pairing/allow-from-store-read.test.ts @@ -0,0 +1,89 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { afterEach, describe, expect, it } from "vitest"; +import { + clearAllowFromStoreReadCacheForTest, + readChannelAllowFromStoreEntriesSync, + resolveChannelAllowFromPath, +} from "./allow-from-store-read.js"; + +const tempDirs: string[] = []; + +function makeEnv(homeDir: string): NodeJS.ProcessEnv { + return { + ...process.env, + HOME: homeDir, + }; +} + +function makeHomeDir(): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-allow-from-read-")); + tempDirs.push(dir); + return dir; +} + +function writeAllowFromFile(params: { + channel: "telegram"; + env: NodeJS.ProcessEnv; + accountId?: string; + allowFrom: string[]; +}): void { + const filePath = resolveChannelAllowFromPath(params.channel, params.env, params.accountId); + fs.mkdirSync(path.dirname(filePath), { recursive: true }); + fs.writeFileSync( + filePath, + JSON.stringify({ version: 1, allowFrom: params.allowFrom }, null, 2), + "utf8", + ); +} + +afterEach(() => { + clearAllowFromStoreReadCacheForTest(); + for (const dir of tempDirs.splice(0)) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe("allow-from-store-read", () => { + it("merges scoped and legacy entries for the default account", () => { + const env = makeEnv(makeHomeDir()); + writeAllowFromFile({ + channel: "telegram", + env, + allowFrom: [" legacy-a ", "legacy-a", "legacy-b"], + }); + writeAllowFromFile({ + channel: "telegram", + env, + accountId: "default", + allowFrom: [" scoped-a ", "legacy-b"], + }); + + expect(readChannelAllowFromStoreEntriesSync("telegram", env)).toEqual([ + "scoped-a", + "legacy-b", + "legacy-a", + ]); + }); + + it("keeps non-default account reads scoped", () => { + const env = makeEnv(makeHomeDir()); + writeAllowFromFile({ + channel: "telegram", + env, + allowFrom: ["legacy-a"], + }); + writeAllowFromFile({ + channel: "telegram", + env, + accountId: "work", + allowFrom: [" work-a ", "work-b"], + }); + + expect(readChannelAllowFromStoreEntriesSync("telegram", env, "work")).toEqual([ + "work-a", + "work-b", + ]); + }); +}); diff --git a/src/pairing/allow-from-store-read.ts b/src/pairing/allow-from-store-read.ts new file mode 100644 index 00000000000..45dce8dac0c --- /dev/null +++ b/src/pairing/allow-from-store-read.ts @@ -0,0 +1,322 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; +import { resolveOAuthDir, resolveStateDir } from "../config/paths.js"; +import { resolveRequiredHomeDir } from "../infra/home-dir.js"; +import { readJsonFileWithFallback } from "../plugin-sdk/json-store.js"; +import { DEFAULT_ACCOUNT_ID } from "../routing/session-key.js"; +import { + normalizeLowercaseStringOrEmpty, + normalizeOptionalString, +} from "../shared/string-coerce.js"; +import type { PairingChannel } from "./pairing-store.types.js"; + +type AllowFromStore = { + version: 1; + allowFrom: string[]; +}; + +type AllowFromReadCacheEntry = { + exists: boolean; + mtimeMs: number | null; + size: number | null; + entries: string[]; +}; + +type AllowFromStatLike = { mtimeMs: number; size: number } | null; + +const allowFromReadCache = new Map(); + +function resolveCredentialsDir(env: NodeJS.ProcessEnv = process.env): string { + const stateDir = resolveStateDir(env, () => resolveRequiredHomeDir(env, os.homedir)); + return resolveOAuthDir(env, stateDir); +} + +function safeChannelKey(channel: PairingChannel): string { + const raw = normalizeLowercaseStringOrEmpty(String(channel)); + if (!raw) { + throw new Error("invalid pairing channel"); + } + const safe = raw.replace(/[\\/:*?"<>|]/g, "_").replace(/\.\./g, "_"); + if (!safe || safe === "_") { + throw new Error("invalid pairing channel"); + } + return safe; +} + +function safeAccountKey(accountId: string): string { + const raw = normalizeLowercaseStringOrEmpty(accountId); + if (!raw) { + throw new Error("invalid pairing account id"); + } + const safe = raw.replace(/[\\/:*?"<>|]/g, "_").replace(/\.\./g, "_"); + if (!safe || safe === "_") { + throw new Error("invalid pairing account id"); + } + return safe; +} + +function resolveAllowFromPath( + channel: PairingChannel, + env: NodeJS.ProcessEnv = process.env, + accountId?: string, +): string { + const base = safeChannelKey(channel); + const normalizedAccountId = normalizeOptionalString(accountId) ?? ""; + if (!normalizedAccountId) { + return path.join(resolveCredentialsDir(env), `${base}-allowFrom.json`); + } + return path.join( + resolveCredentialsDir(env), + `${base}-${safeAccountKey(normalizedAccountId)}-allowFrom.json`, + ); +} + +function dedupePreserveOrder(entries: string[]): string[] { + const seen = new Set(); + const out: string[] = []; + for (const entry of entries) { + const normalized = normalizeOptionalString(entry) ?? ""; + if (!normalized || seen.has(normalized)) { + continue; + } + seen.add(normalized); + out.push(normalized); + } + return out; +} + +function normalizeRawAllowFromList(store: AllowFromStore): string[] { + const list = Array.isArray(store.allowFrom) ? store.allowFrom : []; + return dedupePreserveOrder( + list.map((entry) => normalizeOptionalString(entry) ?? "").filter(Boolean), + ); +} + +function cloneAllowFromCacheEntry(entry: AllowFromReadCacheEntry): AllowFromReadCacheEntry { + return { + exists: entry.exists, + mtimeMs: entry.mtimeMs, + size: entry.size, + entries: entry.entries.slice(), + }; +} + +function setAllowFromReadCache(filePath: string, entry: AllowFromReadCacheEntry): void { + allowFromReadCache.set(filePath, cloneAllowFromCacheEntry(entry)); +} + +function resolveAllowFromReadCacheHit(params: { + filePath: string; + exists: boolean; + mtimeMs: number | null; + size: number | null; +}): AllowFromReadCacheEntry | null { + const cached = allowFromReadCache.get(params.filePath); + if (!cached) { + return null; + } + if (cached.exists !== params.exists) { + return null; + } + if (!params.exists) { + return cloneAllowFromCacheEntry(cached); + } + if (cached.mtimeMs !== params.mtimeMs || cached.size !== params.size) { + return null; + } + return cloneAllowFromCacheEntry(cached); +} + +function resolveAllowFromReadCacheOrMissing( + filePath: string, + stat: AllowFromStatLike, +): { entries: string[]; exists: boolean } | null { + const cached = resolveAllowFromReadCacheHit({ + filePath, + exists: Boolean(stat), + mtimeMs: stat?.mtimeMs ?? null, + size: stat?.size ?? null, + }); + if (cached) { + return { entries: cached.entries, exists: cached.exists }; + } + if (!stat) { + setAllowFromReadCache(filePath, { + exists: false, + mtimeMs: null, + size: null, + entries: [], + }); + return { entries: [], exists: false }; + } + return null; +} + +async function readAllowFromEntriesForPathWithExists( + filePath: string, +): Promise<{ entries: string[]; exists: boolean }> { + let stat: Awaited> | null = null; + try { + stat = await fs.promises.stat(filePath); + } catch (err) { + const code = (err as { code?: string }).code; + if (code !== "ENOENT") { + throw err; + } + } + + const cachedOrMissing = resolveAllowFromReadCacheOrMissing(filePath, stat); + if (cachedOrMissing) { + return cachedOrMissing; + } + if (!stat) { + return { entries: [], exists: false }; + } + + const { value, exists } = await readJsonFileWithFallback(filePath, { + version: 1, + allowFrom: [], + }); + const entries = normalizeRawAllowFromList(value); + setAllowFromReadCache(filePath, { + exists, + mtimeMs: stat.mtimeMs, + size: stat.size, + entries, + }); + return { entries, exists }; +} + +function readAllowFromEntriesForPathSyncWithExists(filePath: string): { + entries: string[]; + exists: boolean; +} { + let stat: fs.Stats | null = null; + try { + stat = fs.statSync(filePath); + } catch (err) { + const code = (err as { code?: string }).code; + if (code !== "ENOENT") { + return { entries: [], exists: false }; + } + } + + const cachedOrMissing = resolveAllowFromReadCacheOrMissing(filePath, stat); + if (cachedOrMissing) { + return cachedOrMissing; + } + if (!stat) { + return { entries: [], exists: false }; + } + + let raw = ""; + try { + raw = fs.readFileSync(filePath, "utf8"); + } catch (err) { + const code = (err as { code?: string }).code; + if (code === "ENOENT") { + return { entries: [], exists: false }; + } + return { entries: [], exists: false }; + } + + try { + const parsed = JSON.parse(raw) as AllowFromStore; + const entries = normalizeRawAllowFromList(parsed); + setAllowFromReadCache(filePath, { + exists: true, + mtimeMs: stat.mtimeMs, + size: stat.size, + entries, + }); + return { entries, exists: true }; + } catch { + setAllowFromReadCache(filePath, { + exists: true, + mtimeMs: stat.mtimeMs, + size: stat.size, + entries: [], + }); + return { entries: [], exists: true }; + } +} + +function shouldIncludeLegacyAllowFromEntries(normalizedAccountId: string): boolean { + return !normalizedAccountId || normalizedAccountId === DEFAULT_ACCOUNT_ID; +} + +function resolveAllowFromAccountId(accountId?: string): string { + return normalizeLowercaseStringOrEmpty(accountId) || DEFAULT_ACCOUNT_ID; +} + +export function resolveChannelAllowFromPath( + channel: PairingChannel, + env: NodeJS.ProcessEnv = process.env, + accountId?: string, +): string { + return resolveAllowFromPath(channel, env, accountId); +} + +export async function readLegacyChannelAllowFromStoreEntries( + channel: PairingChannel, + env: NodeJS.ProcessEnv = process.env, +): Promise { + const filePath = resolveAllowFromPath(channel, env); + return (await readAllowFromEntriesForPathWithExists(filePath)).entries; +} + +export async function readChannelAllowFromStoreEntries( + channel: PairingChannel, + env: NodeJS.ProcessEnv = process.env, + accountId?: string, +): Promise { + const resolvedAccountId = resolveAllowFromAccountId(accountId); + if (!shouldIncludeLegacyAllowFromEntries(resolvedAccountId)) { + return ( + await readAllowFromEntriesForPathWithExists( + resolveAllowFromPath(channel, env, resolvedAccountId), + ) + ).entries; + } + const scopedEntries = ( + await readAllowFromEntriesForPathWithExists( + resolveAllowFromPath(channel, env, resolvedAccountId), + ) + ).entries; + const legacyEntries = ( + await readAllowFromEntriesForPathWithExists(resolveAllowFromPath(channel, env)) + ).entries; + return dedupePreserveOrder([...scopedEntries, ...legacyEntries]); +} + +export function readLegacyChannelAllowFromStoreEntriesSync( + channel: PairingChannel, + env: NodeJS.ProcessEnv = process.env, +): string[] { + return readAllowFromEntriesForPathSyncWithExists(resolveAllowFromPath(channel, env)).entries; +} + +export function readChannelAllowFromStoreEntriesSync( + channel: PairingChannel, + env: NodeJS.ProcessEnv = process.env, + accountId?: string, +): string[] { + const resolvedAccountId = resolveAllowFromAccountId(accountId); + if (!shouldIncludeLegacyAllowFromEntries(resolvedAccountId)) { + return readAllowFromEntriesForPathSyncWithExists( + resolveAllowFromPath(channel, env, resolvedAccountId), + ).entries; + } + const scopedEntries = readAllowFromEntriesForPathSyncWithExists( + resolveAllowFromPath(channel, env, resolvedAccountId), + ).entries; + const legacyEntries = readAllowFromEntriesForPathSyncWithExists( + resolveAllowFromPath(channel, env), + ).entries; + return dedupePreserveOrder([...scopedEntries, ...legacyEntries]); +} + +export function clearAllowFromStoreReadCacheForTest(): void { + allowFromReadCache.clear(); +}