From 08cb567d3745ace4c7159eaaf966c13c04def004 Mon Sep 17 00:00:00 2001 From: Dizzle <112548150+maxmad64bis@users.noreply.github.com> Date: Thu, 6 Aug 2026 03:31:49 +0200 Subject: [PATCH] fix(db): purge orphan proxy_assignments when deleting provider connections (#9246) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Validated in local merge-train (devbox-vm-06-dev002) @ combined-tip (FAST gates: typecheck/complexity/cognitive/changelog/vitest — only pre-existing audit.test.ts flake). Evidence: /home/diegosouzapw/dev/proxys/OmniRoute/.claude/worktrees/merge-train-20260805-222248-suite.log --- .../9246-purge-orphan-proxy-assignments.md | 1 + src/lib/db/providers.ts | 113 ++------------ src/lib/db/providers/deletion.ts | 141 ++++++++++++++++++ ...-purge-proxy-assignments-on-delete.test.ts | 113 ++++++++++++++ 4 files changed, 266 insertions(+), 102 deletions(-) create mode 100644 changelog.d/fixes/9246-purge-orphan-proxy-assignments.md create mode 100644 src/lib/db/providers/deletion.ts create mode 100644 tests/unit/9232-purge-proxy-assignments-on-delete.test.ts diff --git a/changelog.d/fixes/9246-purge-orphan-proxy-assignments.md b/changelog.d/fixes/9246-purge-orphan-proxy-assignments.md new file mode 100644 index 0000000000..ee460f8ca4 --- /dev/null +++ b/changelog.d/fixes/9246-purge-orphan-proxy-assignments.md @@ -0,0 +1 @@ +- **fix(db):** deleting a provider connection (single, batch, or provider-scoped) now purges its account-scoped `proxy_assignments` rows inside an atomic transaction — no more orphan assignments pointing at deleted connections ([#9246](https://github.com/diegosouzapw/OmniRoute/pull/9246)) — thanks @maxmad64bis diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index f5cd1c3c6f..02a39f74c5 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -12,6 +12,7 @@ import { } from "./encryption"; import { createLazyRowProxy } from "./providers/lazyConnectionView"; import { invalidateDbCache, getCachedRawProviderConnections } from "./readCache"; +import { reorderConnections } from "./providers/deletion"; import { removeConnectionHealth, removeConnectionIndex, @@ -119,7 +120,7 @@ export async function getProviderConnections( filter: JsonRecord = {}, limit?: number, offset?: number, - columns?: string[], + columns?: string[] ) { const useCache = !columns?.length && limit === undefined && offset === undefined; const raw = useCache @@ -145,7 +146,7 @@ export async function getRawProviderConnections( filter: JsonRecord = {}, limit?: number, offset?: number, - columns?: string[], + columns?: string[] ) { const db = getDbInstance() as unknown as DbLike; let selectCols = "*"; @@ -177,8 +178,6 @@ export async function getRawProviderConnections( params.authType = filter.authType; } - - if (conditions.length > 0) { sql += " WHERE " + conditions.join(" AND "); } @@ -544,7 +543,7 @@ export async function createProviderConnection(data: JsonRecord) { _insertConnectionRow(db, encryptConnectionFields({ ...connection })); const providerId = toStringOrNull(data.provider); if (providerId) { - _reorderConnections(db, providerId); + reorderConnections(db, providerId); } backupDbFile("pre-write"); invalidateDbCache("connections"); // Bust connections read cache @@ -771,7 +770,7 @@ export async function updateProviderConnection(id: string, data: JsonRecord) { typeof existingRecord.provider === "string" ? existingRecord.provider : String(existingRecord.provider || ""); - _reorderConnections(db, providerId); + reorderConnections(db, providerId); } return withNullableRateLimitOverrides( @@ -898,99 +897,6 @@ export async function resetConnectionBackoff(id: string): Promise { bumpProxyConfigGeneration(); } -export async function deleteProviderConnection(id: string) { - const db = getDbInstance() as unknown as DbLike; - const existing = db.prepare("SELECT provider FROM provider_connections WHERE id = ?").get(id); - if (!existing) return false; - - db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?").run(id); - db.prepare("DELETE FROM provider_connections WHERE id = ?").run(id); - removeConnectionHealth(id); - removeConnectionIndex(id); - bumpProxyConfigGeneration(); - const existingRecord = toRecord(existing); - const providerId = - typeof existingRecord.provider === "string" - ? existingRecord.provider - : String(existingRecord.provider || ""); - _reorderConnections(db, providerId); - backupDbFile("pre-write"); - invalidateDbCache("connections"); // Bust connections read cache - invalidateReasoningRoutingRuleCache(); - return true; -} - -export async function deleteProviderConnections(ids: string[]): Promise { - if (ids.length === 0) return 0; - const db = getDbInstance(); - - const deletedCount = db.transaction(() => { - const placeholders = ids.map(() => "?").join(","); - db.prepare(`DELETE FROM quota_snapshots WHERE connection_id IN (${placeholders})`).run(...ids); - const result = db - .prepare(`DELETE FROM provider_connections WHERE id IN (${placeholders})`) - .run(...ids); - return result.changes ?? 0; - })(); - - for (const id of ids) { - removeConnectionHealth(id); - removeConnectionIndex(id); - } - backupDbFile("pre-write"); - invalidateDbCache("connections"); - invalidateReasoningRoutingRuleCache(); - return deletedCount; -} - -export async function deleteProviderConnectionsByProvider(providerId: string) { - const db = getDbInstance() as unknown as DbLike; - const connectionIds = db - .prepare("SELECT id FROM provider_connections WHERE provider = ?") - .all(providerId) - .map((row) => { - const record = toRecord(row); - return typeof record.id === "string" ? record.id : null; - }) - .filter((id): id is string => id !== null); - - if (connectionIds.length > 0) { - const deleteSnapshots = db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?"); - for (const connectionId of connectionIds) { - deleteSnapshots.run(connectionId); - } - } - - const result = db.prepare("DELETE FROM provider_connections WHERE provider = ?").run(providerId); - for (const connectionId of connectionIds) { - removeConnectionHealth(connectionId); - removeConnectionIndex(connectionId); - } - backupDbFile("pre-write"); - invalidateDbCache("connections"); - invalidateReasoningRoutingRuleCache(); - return result.changes; -} - -export async function reorderProviderConnections(providerId: string) { - const db = getDbInstance() as unknown as DbLike; - _reorderConnections(db, providerId); -} - -function _reorderConnections(db: DbLike, providerId: string) { - const rows = db - .prepare( - "SELECT id, priority, updated_at FROM provider_connections WHERE provider = ? ORDER BY priority ASC, updated_at DESC" - ) - .all(providerId); - - const update = db.prepare("UPDATE provider_connections SET priority = ? WHERE id = ?"); - rows.forEach((row, index) => { - const current = toRecord(row); - update.run(index + 1, current.id); - }); -} - export async function cleanupProviderConnections() { return 0; } @@ -1005,10 +911,13 @@ export async function getDistinctGroups(): Promise { return rows.map((r) => String(r.group ?? "")).filter(Boolean); } +export { autoMigrateLegacyEncryptedConnections, getGheCopilotHosts } from "./providers/migrations"; export { - autoMigrateLegacyEncryptedConnections, - getGheCopilotHosts, -} from "./providers/migrations"; + deleteProviderConnection, + deleteProviderConnections, + deleteProviderConnectionsByProvider, + reorderProviderConnections, +} from "./providers/deletion"; // ──────────────── Re-exports from leaf modules ──────────────── diff --git a/src/lib/db/providers/deletion.ts b/src/lib/db/providers/deletion.ts new file mode 100644 index 0000000000..38848e30df --- /dev/null +++ b/src/lib/db/providers/deletion.ts @@ -0,0 +1,141 @@ +/** + * db/providers/deletion.ts — Provider connection deletion & reordering. + * + * Extracted from db/providers.ts (god-file shrink): the three provider + * connection delete paths plus the connection reorder helper they share. + * Each delete path also purges the connection's account-scoped + * proxy_assignments (#9232) so orphans never keep routing to removed + * connections. + */ + +import { getDbInstance } from "../core"; +import { backupDbFile } from "../backup"; +import { + removeConnectionHealth, + removeConnectionIndex, +} from "@omniroute/open-sse/services/apiKeyRotator.ts"; +import { invalidateDbCache } from "../readCache"; +import { invalidateReasoningRoutingRuleCache } from "../reasoningRoutingRules"; +import { bumpProxyConfigGeneration } from "../settings"; +import { toRecord } from "./columns"; + +interface StatementLike { + all: (...params: unknown[]) => TRow[]; + get: (...params: unknown[]) => TRow | undefined; + run: (...params: unknown[]) => { changes?: number }; +} + +interface DbLike { + prepare: (sql: string) => StatementLike; + transaction: (fn: () => T) => () => T; +} + +// Purge account-scoped proxy_assignments for the deleted connections (#9232): +// orphan assignments otherwise keep routing to provider_connections that no +// longer exist. scope='account' lives in exactly this one place. +function _deleteAccountProxyAssignments(db: DbLike, ids: string[]) { + if (ids.length === 0) return; + const placeholders = ids.map(() => "?").join(","); + db.prepare( + `DELETE FROM proxy_assignments WHERE scope = 'account' AND scope_id IN (${placeholders})` + ).run(...ids); +} + +export async function deleteProviderConnection(id: string) { + const db = getDbInstance() as unknown as DbLike; + const existing = db.prepare("SELECT provider FROM provider_connections WHERE id = ?").get(id); + if (!existing) return false; + + db.transaction(() => { + _deleteAccountProxyAssignments(db, [id]); + db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?").run(id); + db.prepare("DELETE FROM provider_connections WHERE id = ?").run(id); + })(); + removeConnectionHealth(id); + removeConnectionIndex(id); + bumpProxyConfigGeneration(); + const existingRecord = toRecord(existing); + const providerId = + typeof existingRecord.provider === "string" + ? existingRecord.provider + : String(existingRecord.provider || ""); + reorderConnections(db, providerId); + backupDbFile("pre-write"); + invalidateDbCache("connections"); // Bust connections read cache + invalidateReasoningRoutingRuleCache(); + return true; +} + +export async function deleteProviderConnections(ids: string[]): Promise { + if (ids.length === 0) return 0; + const db = getDbInstance() as unknown as DbLike; + + const deletedCount = db.transaction(() => { + const placeholders = ids.map(() => "?").join(","); + db.prepare(`DELETE FROM quota_snapshots WHERE connection_id IN (${placeholders})`).run(...ids); + _deleteAccountProxyAssignments(db, ids); + const result = db + .prepare(`DELETE FROM provider_connections WHERE id IN (${placeholders})`) + .run(...ids); + return result.changes ?? 0; + })(); + + for (const id of ids) { + removeConnectionHealth(id); + removeConnectionIndex(id); + } + backupDbFile("pre-write"); + invalidateDbCache("connections"); + invalidateReasoningRoutingRuleCache(); + return deletedCount; +} + +export async function deleteProviderConnectionsByProvider(providerId: string) { + const db = getDbInstance() as unknown as DbLike; + const connectionIds = db + .prepare("SELECT id FROM provider_connections WHERE provider = ?") + .all(providerId) + .map((row) => { + const record = toRecord(row); + return typeof record.id === "string" ? record.id : null; + }) + .filter((id): id is string => id !== null); + + const result = db.transaction(() => { + if (connectionIds.length > 0) { + const deleteSnapshots = db.prepare("DELETE FROM quota_snapshots WHERE connection_id = ?"); + for (const connectionId of connectionIds) { + deleteSnapshots.run(connectionId); + } + _deleteAccountProxyAssignments(db, connectionIds); + } + return db.prepare("DELETE FROM provider_connections WHERE provider = ?").run(providerId); + })(); + for (const connectionId of connectionIds) { + removeConnectionHealth(connectionId); + removeConnectionIndex(connectionId); + } + backupDbFile("pre-write"); + invalidateDbCache("connections"); + invalidateReasoningRoutingRuleCache(); + return result.changes; +} + +export async function reorderProviderConnections(providerId: string) { + const db = getDbInstance() as unknown as DbLike; + reorderConnections(db, providerId); +} + +export function reorderConnections(db: DbLike, providerId: string) { + const rows = db + .prepare( + "SELECT id, priority, updated_at FROM provider_connections WHERE provider = ? ORDER BY priority ASC, updated_at DESC" + ) + .all(providerId); + + const update = db.prepare("UPDATE provider_connections SET priority = ? WHERE id = ?"); + rows.forEach((row, index) => { + const current = toRecord(row); + update.run(index + 1, current.id); + }); +} diff --git a/tests/unit/9232-purge-proxy-assignments-on-delete.test.ts b/tests/unit/9232-purge-proxy-assignments-on-delete.test.ts new file mode 100644 index 0000000000..aad652d005 --- /dev/null +++ b/tests/unit/9232-purge-proxy-assignments-on-delete.test.ts @@ -0,0 +1,113 @@ +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-9232-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const TEST_PROVIDER = "__test_provider_9232__"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const proxiesDb = await import("../../src/lib/db/proxies.ts"); + +async function resetStorage() { + core.resetDbInstance(); + for (let attempt = 0; attempt < 10; attempt++) { + try { + if (fs.existsSync(TEST_DATA_DIR)) fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + break; + } catch (error: unknown) { + const code = (error as { code?: string } | undefined)?.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(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +async function setupConnectionWithAssignment() { + const created = await providersDb.createProviderConnection({ + provider: TEST_PROVIDER, + authType: "apikey", + name: `Test conn ${Date.now()}-${Math.random().toString(36).slice(2, 7)}`, + apiKey: `sk-test-9232-${Math.random().toString(36).slice(2, 9)}`, + }); + assert.ok(created?.id, "connection must be created"); + const proxy = await proxiesDb.createProxy({ + name: "Test proxy 9232", + type: "http", + host: "proxy.local", + port: 8080, + }); + assert.ok(proxy?.id, "proxy must be created"); + const assignment = await proxiesDb.assignProxyToScope("account", created.id, proxy.id); + assert.ok(assignment, "assignment must be created"); + return created as { id: string }; +} + +async function getAccountAssignmentsFor(connectionId: string) { + const all = await proxiesDb.getProxyAssignments({ scope: "account" }); + return all.filter((a) => a.scopeId === connectionId); +} + +test("#9232: deleteProviderConnection purges account proxy_assignments for the deleted connection", async () => { + const conn = await setupConnectionWithAssignment(); + const before = await getAccountAssignmentsFor(conn.id); + assert.equal(before.length, 1, "sanity: assignment exists before delete"); + + const deleted = await providersDb.deleteProviderConnection(conn.id); + assert.equal(deleted, true, "connection row must be deleted"); + + const after = await getAccountAssignmentsFor(conn.id); + assert.equal(after.length, 0, "EXPECTED: proxy_assignments rows purged with the connection"); +}); + +test("#9232: deleteProviderConnections purges account proxy_assignments for the whole batch", async () => { + const connA = await setupConnectionWithAssignment(); + const connB = await setupConnectionWithAssignment(); + assert.equal((await getAccountAssignmentsFor(connA.id)).length, 1); + assert.equal((await getAccountAssignmentsFor(connB.id)).length, 1); + + const deleted = await providersDb.deleteProviderConnections([connA.id, connB.id]); + assert.equal(deleted, 2, "both connections deleted"); + + assert.equal((await getAccountAssignmentsFor(connA.id)).length, 0); + assert.equal((await getAccountAssignmentsFor(connB.id)).length, 0); +}); + +test("#9232: deleteProviderConnectionsByProvider purges account proxy_assignments for all provider connections", async () => { + const connA = await setupConnectionWithAssignment(); + const connB = await setupConnectionWithAssignment(); + assert.equal((await getAccountAssignmentsFor(connA.id)).length, 1); + assert.equal((await getAccountAssignmentsFor(connB.id)).length, 1); + + const deleted = await providersDb.deleteProviderConnectionsByProvider(TEST_PROVIDER); + assert.equal(deleted, 2, "both provider connections deleted"); + + assert.equal((await getAccountAssignmentsFor(connA.id)).length, 0); + assert.equal((await getAccountAssignmentsFor(connB.id)).length, 0); +}); + +test("#9232: deleteProviderConnections with an empty batch is a no-op and stays valid SQL", async () => { + const deleted = await providersDb.deleteProviderConnections([]); + assert.equal(deleted, 0, "empty batch deletes nothing"); +}); + +test("#9232: deleteProviderConnectionsByProvider with no matching connections purges nothing and returns 0", async () => { + const deleted = await providersDb.deleteProviderConnectionsByProvider( + "__no_such_provider_9232__" + ); + assert.equal(deleted, 0, "no provider connections matched, nothing to delete"); +});