mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-09-14 19:02:17 +03:00
fix(db): ignore non-finite rate_limited_until writes, preserve null clear (#12788)
Validado numa worktree combinada com a onda de persistência desta leva sobre `release/v3.8.51`: check-file-size e check-changelog-integrity OK, typecheck:core limpo, check-api-typecheck OK (289, dentro da baseline), 70 testes focados no runner Node e 1 no vitest, todos verdes. Persistir `NaN`/`Infinity` numa coluna TEXT envenena toda leitura futura, e um timestamp já expirado sobrescrevendo uma linha viva é pior que não escrever nada. O guard fundido na cabeça da função cobre os dois sem tocar leitores nem o caminho de clear. Os 6 casos do teste incluem o que mais importa: `null` continua limpando, e uma escrita expirada não derruba um cooldown ativo. Nota: 3 deles falham antes do guard, como você registrou. **Integração:** este arquivo colidiu com o #12951, que também guarda a cabeça de `setConnectionRateLimitUntil` — lá o `null` vira caminho de clear que também remove os cooldowns filhos do Codex. Os dois compõem: trata-se o `null` primeiro (clear + return), e o seu guard de finitude/expiração passa a valer para os não-nulos. Ambos preservados.
This commit is contained in:
1
changelog.d/fixes/12788-ratelimit-write-guard.md
Normal file
1
changelog.d/fixes/12788-ratelimit-write-guard.md
Normal file
@@ -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
|
||||
@@ -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 = ?"
|
||||
|
||||
121
tests/unit/db-rate-limit-guard.test.ts
Normal file
121
tests/unit/db-rate-limit-guard.test.ts
Normal file
@@ -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<string> {
|
||||
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.
|
||||
Reference in New Issue
Block a user