diff --git a/changelog.d/fixes/13165-telegram-keycache-unbounded.md b/changelog.d/fixes/13165-telegram-keycache-unbounded.md new file mode 100644 index 0000000000..2672fb93be --- /dev/null +++ b/changelog.d/fixes/13165-telegram-keycache-unbounded.md @@ -0,0 +1 @@ +- **fix(telegram):** bound the per-user API key cache in the Telegram chat proxy so a burst of distinct chat ids can no longer grow the process heap without limit ([#13165](https://github.com/diegosouzapw/OmniRoute/issues/13165)) diff --git a/src/lib/telegram/chatProxy.ts b/src/lib/telegram/chatProxy.ts index d2b136954e..724ccd2b00 100644 --- a/src/lib/telegram/chatProxy.ts +++ b/src/lib/telegram/chatProxy.ts @@ -21,11 +21,31 @@ const DEFAULT_MODEL = process.env.TELEGRAM_DEFAULT_MODEL || "auto/chat"; * Resolve (and lazily mint) an OmniRoute API key for a Telegram user. * Returns the plaintext key value, cached per user id. */ +// Bounded LRU. The webhook path passes a caller-supplied chat id, so the key +// space is not limited to the real user population and an uncapped Map would +// grow for the lifetime of the process. Insertion order is the recency order: +// a hit re-inserts, and the oldest entry is dropped once the cap is reached. +const KEY_CACHE_MAX_ENTRIES = 1000; const keyCache = new Map(); +function rememberUserApiKey(telegramUserId: number, key: string): void { + // Re-insert so this id becomes the most recently used entry. + keyCache.delete(telegramUserId); + keyCache.set(telegramUserId, key); + while (keyCache.size > KEY_CACHE_MAX_ENTRIES) { + const oldest = keyCache.keys().next(); + if (oldest.done) break; + keyCache.delete(oldest.value); + } +} + export async function resolveUserApiKey(telegramUserId: number): Promise { const cached = keyCache.get(telegramUserId); - if (cached) return cached; + if (cached) { + // Refresh recency so an active user is not evicted by a burst of new ids. + rememberUserApiKey(telegramUserId, cached); + return cached; + } const machineId = (await getConsistentMachineId().catch(() => null)) || "0000000000000000"; @@ -39,12 +59,12 @@ export async function resolveUserApiKey(telegramUserId: number): Promise ); const matchKey = (match as { key?: string } | undefined)?.key; if (typeof matchKey === "string" && matchKey.length > 0) { - keyCache.set(telegramUserId, matchKey); + rememberUserApiKey(telegramUserId, matchKey); return matchKey; } const created = await createApiKey(`telegram:${telegramUserId}`, machineId); - keyCache.set(telegramUserId, created.key); + rememberUserApiKey(telegramUserId, created.key); return created.key; } diff --git a/tests/unit/telegram-keycache-bounded-13165.test.ts b/tests/unit/telegram-keycache-bounded-13165.test.ts new file mode 100644 index 0000000000..46305744c5 --- /dev/null +++ b/tests/unit/telegram-keycache-bounded-13165.test.ts @@ -0,0 +1,123 @@ +/** + * Regression test for #13165: the Telegram per-user key cache must stay bounded. + * + * `resolveUserApiKey()` is reachable from the webhook path of + * POST /api/telegram/update with a caller-supplied chat id, so an uncapped Map + * grows for the lifetime of the process. The cache is module-private, so this + * asserts the observable LRU contract: a cold id is re-minted after a burst of + * distinct ids (proving eviction), while a recently used id survives it. + * + * Runner: node:test (tests/unit/*.test.ts), so DB access is stubbed through a + * module mock rather than vi.mock. + */ +import { test, describe, before, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { register } from "node:module"; +import { pathToFileURL } from "node:url"; + +const CAP = 1000; + +/** Names passed to createApiKey — one entry per real mint (i.e. per cache miss). */ +const minted: string[] = []; + +let resolveUserApiKey: (id: number) => Promise; + +before(async () => { + // Stub the DB + machine-id modules so nothing touches SQLite. The loader + // matches the specifiers used by chatProxy.ts. The stub must export every + // name the real module exports: chatProxy pulls in the chat handler, which + // imports other members of this module, and a missing export is a module-load + // SyntaxError that would look like a failing assertion. + const dbExports = [ + "clearApiKeyCaches", + "deleteApiKey", + "getApiKeyById", + "getApiKeyMetadata", + "getApiKeysCount", + "getExclusiveLeaseConnectionIds", + "isModelAllowedForKey", + "pickApiKeyForInternalUse", + "regenerateApiKey", + "resetApiKeyState", + "revokeApiKey", + "setApiKeyExpiry", + "updateApiKeyPermissions", + "validateApiKey", + ]; + + const dbStub = ` + export async function getApiKeys() { return []; } + export async function createApiKey(name) { + globalThis.__mintedKeys.push(name); + return { key: "sk-omni-" + "x".repeat(32) + "-" + name }; + } + ${dbExports.map((n) => `export async function ${n}() { return null; }`).join("\n")} + `; + const machineStub = ` + export async function getConsistentMachineId() { return "0000000000000000"; } + `; + + (globalThis as Record).__mintedKeys = minted; + + const loader = ` + export async function resolve(spec, ctx, next) { + if (spec.includes("db/apiKeys")) { + return { url: "data:text/javascript,${encodeURIComponent(dbStub)}", shortCircuit: true }; + } + if (spec.includes("machineId")) { + return { url: "data:text/javascript,${encodeURIComponent(machineStub)}", shortCircuit: true }; + } + return next(spec, ctx); + } + `; + register("data:text/javascript," + encodeURIComponent(loader), pathToFileURL("./")); + + ({ resolveUserApiKey } = await import("../../src/lib/telegram/chatProxy.ts")); +}); + +describe("telegram keyCache bounding (#13165)", () => { + beforeEach(() => { + minted.length = 0; + }); + + test("evicts a cold id once the cap is exceeded", async () => { + const victim = 7_000_001; + const beforeFirstResolve = minted.length; + await resolveUserApiKey(victim); + assert.equal(minted.length - beforeFirstResolve, 1, "first resolve should mint exactly once"); + + // Never touch `victim` again: it must fall out of a CAP-sized cache. + for (let i = 0; i < CAP + 50; i++) await resolveUserApiKey(600_000 + i); + + // Measure the victim's own resolve in isolation. Comparing against the + // running total would be dominated by the burst's own mints and would pass + // even with an unbounded cache. + const beforeVictimResolve = minted.length; + await resolveUserApiKey(victim); + const mintedForVictim = minted.length - beforeVictimResolve; + + // Evicted => cache miss => exactly one fresh mint for this id. + assert.equal( + mintedForVictim, + 1, + `expected victim to be re-minted after eviction, got ${mintedForVictim} mint(s)` + ); + }); + + test("keeps a recently used id alive across a burst of new ids", async () => { + const active = 8_000_001; + const first = await resolveUserApiKey(active); + + // Touch the active id throughout the burst so it stays most-recently-used. + for (let i = 0; i < CAP * 2; i++) { + await resolveUserApiKey(500_000 + i); + if (i % 100 === 0) await resolveUserApiKey(active); + } + + const mintsBefore = minted.length; + const again = await resolveUserApiKey(active); + + assert.equal(again, first, "active id should keep its cached key"); + assert.equal(minted.length, mintsBefore, "active id should not be re-minted"); + }); +});