diff --git a/changelog.d/fixes/12788-ratelimit-write-guard.md b/changelog.d/fixes/12788-ratelimit-write-guard.md new file mode 100644 index 0000000000..1662233abf --- /dev/null +++ b/changelog.d/fixes/12788-ratelimit-write-guard.md @@ -0,0 +1 @@ +- **fix(db):** ignore expired or invalid rate-limit cooldown writes so a stale timestamp can't lock a connection that should be usable — clearing still works as before ([#12788](https://github.com/diegosouzapw/OmniRoute/pull/12788)) — thanks @maxmad64bis diff --git a/src/lib/db/providers/rateLimit.ts b/src/lib/db/providers/rateLimit.ts index 7812080a0f..e89199c3b6 100644 --- a/src/lib/db/providers/rateLimit.ts +++ b/src/lib/db/providers/rateLimit.ts @@ -28,6 +28,11 @@ interface DbLike { * @param until - Epoch ms when the rate limit expires (null to clear) */ export function setConnectionRateLimitUntil(connectionId: string, until: number | null): void { + // Guard: never persist a non-finite or already-expired timestamp. The TEXT + // column would store "NaN"/"Infinity" and pollute every future read. null + // is the only clear path (via clearConnectionRateLimit); past/zero + // timestamps are noops so an expired write cannot overwrite a live row. + if (until !== null && (!Number.isFinite(until) || until <= Date.now())) return; const db = getDbInstance() as unknown as DbLike; db.prepare( "UPDATE provider_connections SET rate_limited_until = ?, updated_at = ? WHERE id = ?" diff --git a/tests/unit/db-rate-limit-guard.test.ts b/tests/unit/db-rate-limit-guard.test.ts new file mode 100644 index 0000000000..69fb9e1e06 --- /dev/null +++ b/tests/unit/db-rate-limit-guard.test.ts @@ -0,0 +1,121 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-db-ratelimit-guard-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.ts") as typeof import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts") as typeof import("../../src/lib/db/providers.ts"); +const { + setConnectionRateLimitUntil, + clearConnectionRateLimit, + isConnectionRateLimited, +} = providersDb; + +function readRateLimitedUntil(connectionId: string): unknown { + const db = ( + core as unknown as { + getDbInstance: () => { + prepare: (sql: string) => { + get: (id: string) => { rate_limited_until: unknown } | undefined; + }; + }; + } + ).getDbInstance(); + return db + .prepare("SELECT rate_limited_until FROM provider_connections WHERE id = ?") + .get(connectionId)?.rate_limited_until ?? null; +} + +async function resetStorage() { + core.resetDbInstance(); + // Retry loop copied from db-providers-crud.test.ts:17-30 (Windows EBUSY/EPERM). + for (let attempt = 0; attempt < 10; attempt++) { + try { + if (fs.existsSync(TEST_DATA_DIR)) { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + } + break; + } catch (error: unknown) { + const code = (error as { code?: string } | null)?.code; + if ((code === "EBUSY" || code === "EPERM") && attempt < 9) { + await new Promise((resolve) => setTimeout(resolve, 50 * (attempt + 1))); + } else { + throw error; + } + } + } + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + core.resetDbInstance(); + // No retry here (unlike resetStorage): teardown-only, Linux CI; Windows + // EBUSY surfaces in beforeEach retries, not here. + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +async function seedConnection(): Promise { + const connection = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "Guard probe", + apiKey: "guard-key", + }); + return (connection as { id: string }).id; +} + +test("ignores NaN (row unchanged)", async () => { + const id = await seedConnection(); + setConnectionRateLimitUntil(id, NaN); + assert.equal(readRateLimitedUntil(id), null); +}); + +test("ignores Infinity and -Infinity", async () => { + const id = await seedConnection(); + setConnectionRateLimitUntil(id, Infinity); + setConnectionRateLimitUntil(id, -Infinity); + assert.equal(readRateLimitedUntil(id), null); +}); + +test("ignores past timestamps and 0 (documented noop; clear path is null)", async () => { + const id = await seedConnection(); + setConnectionRateLimitUntil(id, Date.now() - 1000); + setConnectionRateLimitUntil(id, 0); + assert.equal(readRateLimitedUntil(id), null); +}); + +test("writes future timestamps", async () => { + const id = await seedConnection(); + const until = Date.now() + 60_000; + setConnectionRateLimitUntil(id, until); + assert.equal(Number(readRateLimitedUntil(id)), until); + assert.equal(isConnectionRateLimited(id), true); +}); + +test("preserves null clear (non-regression for clearConnectionRateLimit)", async () => { + const id = await seedConnection(); + setConnectionRateLimitUntil(id, Date.now() + 60_000); + clearConnectionRateLimit(id); + assert.equal(readRateLimitedUntil(id), null); + assert.equal(isConnectionRateLimited(id), false); +}); + +test("expired write does not overwrite a live row (future preserved)", async () => { + const id = await seedConnection(); + const future = Date.now() + 60_000; + setConnectionRateLimitUntil(id, future); + setConnectionRateLimitUntil(id, Date.now() - 1000); + assert.equal(Number(readRateLimitedUntil(id)), future); + assert.equal(isConnectionRateLimited(id), true); +}); + +// Without the guard the TEXT column stores the string "NaN" — rejected by +// current readers, but hygiene demands never writing it.