From cba636b9f03a90197c4dccb5c7760029cdd47821 Mon Sep 17 00:00:00 2001 From: Hamsa_M <116961508+hamsa0x7@users.noreply.github.com> Date: Fri, 26 Jun 2026 01:09:55 +0530 Subject: [PATCH] fix(db): replace Math.random with crypto.randomUUID for ID generation (#5026) Replace weak Math.random-based ID generation with crypto.randomUUID() in quota pool, quota group, and migration runner FTS5 probe table name generation. Also remove unnecessary typeof-gated fallback that only ran on ancient Node versions. --- src/lib/db/migrationRunner.ts | 2 +- src/lib/db/quotaGroups.ts | 17 ++------ src/lib/db/quotaPools.ts | 63 +++++++++++++++++----------- tests/unit/db/weak-rng-fixes.test.ts | 55 ++++++++++++++++++++++++ 4 files changed, 98 insertions(+), 39 deletions(-) create mode 100644 tests/unit/db/weak-rng-fixes.test.ts diff --git a/src/lib/db/migrationRunner.ts b/src/lib/db/migrationRunner.ts index 7acfc7c666..f34d9831e2 100644 --- a/src/lib/db/migrationRunner.ts +++ b/src/lib/db/migrationRunner.ts @@ -264,7 +264,7 @@ function supportsFts5(db: SqliteAdapter): boolean { } try { - const probeTable = `__omniroute_fts5_probe_${Date.now()}_${Math.random().toString(36).slice(2)}`; + const probeTable = `__omniroute_fts5_probe_${crypto.randomUUID().replace(/-/g, "_")}`; db.transaction(() => { db.exec(`CREATE VIRTUAL TABLE "${probeTable}" USING fts5(content);`); db.exec(`DROP TABLE "${probeTable}";`); diff --git a/src/lib/db/quotaGroups.ts b/src/lib/db/quotaGroups.ts index 549cebd6df..b944828394 100644 --- a/src/lib/db/quotaGroups.ts +++ b/src/lib/db/quotaGroups.ts @@ -53,10 +53,7 @@ function rowToGroup(row: GroupRow): QuotaGroup { } function makeId(): string { - if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") { - return crypto.randomUUID(); - } - return Date.now().toString(36) + "-" + Math.random().toString(36).slice(2); + return crypto.randomUUID(); } // --------------------------------------------------------------------------- @@ -115,9 +112,7 @@ export function listGroups(): QuotaGroup[] { * Returns true if the row was updated, false if the group was not found. */ export function renameGroup(id: string, name: string): boolean { - const result = getDb() - .prepare("UPDATE quota_groups SET name = ? WHERE id = ?") - .run(name, id); + const result = getDb().prepare("UPDATE quota_groups SET name = ? WHERE id = ?").run(name, id); return result.changes > 0; } @@ -144,13 +139,9 @@ export function deleteGroup(id: string): boolean { .prepare<{ cnt: number }>("SELECT COUNT(*) AS cnt FROM quota_pools WHERE group_id = ?") .get(id); if (refRow && refRow.cnt > 0) { - throw new Error( - `Group '${id}' has pools; reassign or delete them first.` - ); + throw new Error(`Group '${id}' has pools; reassign or delete them first.`); } - const result = getDb() - .prepare("DELETE FROM quota_groups WHERE id = ?") - .run(id); + const result = getDb().prepare("DELETE FROM quota_groups WHERE id = ?").run(id); return result.changes > 0; } diff --git a/src/lib/db/quotaPools.ts b/src/lib/db/quotaPools.ts index 93cf86264d..7343cd0438 100644 --- a/src/lib/db/quotaPools.ts +++ b/src/lib/db/quotaPools.ts @@ -28,7 +28,10 @@ async function removeQuotaCombosGuarded(poolId: string): Promise { const { removeQuotaCombosForPool } = await import("@/lib/quota/quotaCombos"); await removeQuotaCombosForPool(poolId); } catch (err) { - console.warn("[quota-pools] removeQuotaCombosForPool failed (non-fatal):", (err as Error)?.message); + console.warn( + "[quota-pools] removeQuotaCombosForPool failed (non-fatal):", + (err as Error)?.message + ); } } @@ -115,9 +118,9 @@ function assertSingleProvider(connectionIds: string[]): void { const db = getDb(); const placeholders = connectionIds.map(() => "?").join(","); const rows = db - .prepare<{ provider: string }>( - `SELECT DISTINCT provider FROM provider_connections WHERE id IN (${placeholders})` - ) + .prepare<{ + provider: string; + }>(`SELECT DISTINCT provider FROM provider_connections WHERE id IN (${placeholders})`) .all(...connectionIds); const providers = rows.map((r) => r.provider).filter(Boolean); if (new Set(providers).size > 1) { @@ -208,12 +211,7 @@ function getAllocations(poolId: string): PoolAllocation[] { } function makeId(): string { - // Use Web Crypto UUID (available in Node ≥19 globally; also available in browsers) - if (typeof crypto !== "undefined" && typeof crypto.randomUUID === "function") { - return crypto.randomUUID(); - } - // Fallback: timestamp + random (extremely unlikely to collide in tests) - return Date.now().toString(36) + "-" + Math.random().toString(36).slice(2); + return crypto.randomUUID(); } // --------------------------------------------------------------------------- @@ -250,7 +248,9 @@ export function listPools(): QuotaPool[] { */ export function getPool(id: string): QuotaPool | null { const row = getDb() - .prepare("SELECT id, connection_id, name, group_id, created_at FROM quota_pools WHERE id = ?") + .prepare( + "SELECT id, connection_id, name, group_id, created_at FROM quota_pools WHERE id = ?" + ) .get(id); if (!row) return null; return rowToPool(row, getAllocations(row.id)); @@ -282,7 +282,9 @@ export function createPool(input: PoolCreate): QuotaPool { const database = getDb(); const doCreate = database.transaction(() => { database - .prepare("INSERT INTO quota_pools (id, connection_id, name, group_id, created_at) VALUES (?, ?, ?, ?, ?)") + .prepare( + "INSERT INTO quota_pools (id, connection_id, name, group_id, created_at) VALUES (?, ?, ?, ?, ?)" + ) .run(id, primaryConnectionId, input.name, groupId, now); const insertConn = database.prepare( @@ -312,7 +314,13 @@ export function createPool(input: PoolCreate): QuotaPool { doCreate(); const result = rowToPool( - { id, connection_id: primaryConnectionId, name: input.name, group_id: groupId, created_at: now }, + { + id, + connection_id: primaryConnectionId, + name: input.name, + group_id: groupId, + created_at: now, + }, getAllocations(id) ); @@ -331,7 +339,9 @@ export function createPool(input: PoolCreate): QuotaPool { export function updatePool(id: string, input: PoolUpdate): QuotaPool | null { const database = getDb(); const existing = database - .prepare("SELECT id, connection_id, name, group_id, created_at FROM quota_pools WHERE id = ?") + .prepare( + "SELECT id, connection_id, name, group_id, created_at FROM quota_pools WHERE id = ?" + ) .get(id); if (!existing) return null; @@ -354,9 +364,7 @@ export function updatePool(id: string, input: PoolUpdate): QuotaPool | null { if (input.connectionIds !== undefined && input.connectionIds.length > 0) { const newPrimary = input.connectionIds[0]; // Replace join rows. - database - .prepare("DELETE FROM quota_pool_connections WHERE pool_id = ?") - .run(id); + database.prepare("DELETE FROM quota_pool_connections WHERE pool_id = ?").run(id); const insertConn = database.prepare( "INSERT OR IGNORE INTO quota_pool_connections (pool_id, connection_id) VALUES (?, ?)" ); @@ -364,9 +372,7 @@ export function updatePool(id: string, input: PoolUpdate): QuotaPool | null { insertConn.run(id, connId); } // Sync primary column. - database - .prepare("UPDATE quota_pools SET connection_id = ? WHERE id = ?") - .run(newPrimary, id); + database.prepare("UPDATE quota_pools SET connection_id = ? WHERE id = ?").run(newPrimary, id); existing.connection_id = newPrimary; } @@ -413,13 +419,15 @@ export function deletePool(id: string): boolean { const doDelete = database.transaction(() => { database.prepare("DELETE FROM quota_pool_connections WHERE pool_id = ?").run(id); // Prune this pool id from every key's allowed_quotas JSON array. - database.prepare( - `UPDATE api_keys SET allowed_quotas = COALESCE( + database + .prepare( + `UPDATE api_keys SET allowed_quotas = COALESCE( (SELECT json_group_array(value) FROM json_each(api_keys.allowed_quotas) WHERE value != ?), '[]') WHERE allowed_quotas IS NOT NULL AND allowed_quotas != '[]' AND EXISTS (SELECT 1 FROM json_each(api_keys.allowed_quotas) WHERE value = ?)` - ).run(id, id); + ) + .run(id, id); return database.prepare("DELETE FROM quota_pools WHERE id = ?").run(id); }); const result = doDelete(); @@ -467,7 +475,10 @@ export function upsertAllocations(poolId: string, allocations: PoolAllocation[]) // Normalize: when all weights are 0, distribute equally so the pool is usable // without requiring a manual re-save. Persists the normalized weights. - const totalWeight = allocations.reduce((s, a) => s + (Number.isFinite(a.weight) ? a.weight : 0), 0); + const totalWeight = allocations.reduce( + (s, a) => s + (Number.isFinite(a.weight) ? a.weight : 0), + 0 + ); const normalizedAllocations = totalWeight === 0 && allocations.length > 0 ? allocations.map((a) => ({ ...a, weight: 100 / allocations.length })) @@ -476,7 +487,9 @@ export function upsertAllocations(poolId: string, allocations: PoolAllocation[]) // Resolve the target pool's group so we can propagate to siblings. // Defensive: fall back to [poolId] (single-pool semantics) if pool not found. const targetPool = database - .prepare("SELECT id, connection_id, name, group_id, created_at FROM quota_pools WHERE id = ?") + .prepare( + "SELECT id, connection_id, name, group_id, created_at FROM quota_pools WHERE id = ?" + ) .get(poolId); // Collect all pools in the group (includes the target pool itself). diff --git a/tests/unit/db/weak-rng-fixes.test.ts b/tests/unit/db/weak-rng-fixes.test.ts new file mode 100644 index 0000000000..3d23d5d20c --- /dev/null +++ b/tests/unit/db/weak-rng-fixes.test.ts @@ -0,0 +1,55 @@ +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-weak-rng-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +test.after(() => { + try { + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + } catch {} +}); + +test("quotaPools.makeId returns UUID without Math.random fallback", async () => { + const { createPool } = await import("../../../src/lib/db/quotaPools.ts"); + const core = await import("../../../src/lib/db/core.ts"); + core.resetDbInstance(); + + const pool = createPool({ connectionId: "test-conn", name: "test-pool" }); + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + assert.match(pool.id, uuidRegex, `ID should be UUID format, got: ${pool.id}`); +}); + +test("quotaGroups.makeId returns UUID without Math.random fallback", async () => { + const { createGroup } = await import("../../../src/lib/db/quotaGroups.ts"); + const core = await import("../../../src/lib/db/core.ts"); + core.resetDbInstance(); + + const group = createGroup("test-group"); + const uuidRegex = /^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i; + assert.match(group.id, uuidRegex, `ID should be UUID format, got: ${group.id}`); +}); + +test("quotaPools.makeId produces unique IDs across calls", async () => { + const { createPool } = await import("../../../src/lib/db/quotaPools.ts"); + const core = await import("../../../src/lib/db/core.ts"); + + const ids = new Set(); + for (let i = 0; i < 10; i++) { + core.resetDbInstance(); + const pool = createPool({ connectionId: "test-conn", name: `pool-${i}` }); + ids.add(pool.id); + } + assert.equal(ids.size, 10, "All 10 IDs should be unique"); +}); + +test("migrationRunner probe table format uses crypto.randomUUID", async () => { + const probeName = `__omniroute_fts5_probe_${crypto.randomUUID().replace(/-/g, "_")}`; + assert.match( + probeName, + /^__omniroute_fts5_probe_[0-9a-f]{8}_[0-9a-f]{4}_[0-9a-f]{4}_[0-9a-f]{4}_[0-9a-f]{12}$/i + ); +});