mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 02:42:24 +03:00
fix(telegram): bound the per-user API key cache (#13166)
`resolveUserApiKey()` keyed an uncapped Map on an id taken straight from the webhook body. The LRU's recency test is what keeps this from regressing into a clear-when-full cache. --- Validated in one consolidated worktree cut from `release/v3.8.51`, boarded together with the other 13 PRs of this batch — zero merge conflicts between them. - `typecheck:core` clean - complexity 2799 / baseline 3218 and cognitive-complexity 1265 / baseline 1437 — both under baseline - 71 focused assertions green across the 13 test files this batch adds or touches ⚠️ base-red inherited: #12732 — `Docs Gates (fast-path)`, `Merge integrity`, `No new ESLint warnings`, `Unit Tests fast-path` and `Fast Quality Gates` all reproduce on the pure `release/v3.8.51` tip (provider count 356 vs the 358 the modules define, SKILL.md drift, and `open-sse/utils/stream.ts` at 3115 > frozen 3098). None of them touch this diff. Thanks @anhtahaylove — the root-cause write-up, the measured before/after numbers and the red-before-green proof on every one of these made the batch reviewable as a unit.
This commit is contained in:
1
changelog.d/fixes/13165-telegram-keycache-unbounded.md
Normal file
1
changelog.d/fixes/13165-telegram-keycache-unbounded.md
Normal file
@@ -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))
|
||||
@@ -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<number, string>();
|
||||
|
||||
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<string> {
|
||||
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<string>
|
||||
);
|
||||
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;
|
||||
}
|
||||
|
||||
|
||||
123
tests/unit/telegram-keycache-bounded-13165.test.ts
Normal file
123
tests/unit/telegram-keycache-bounded-13165.test.ts
Normal file
@@ -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<string>;
|
||||
|
||||
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<string, unknown>).__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");
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user