From 589dbde2e6e2aeeeee457b4425b3fe87e16e5a72 Mon Sep 17 00:00:00 2001 From: Ronaldo Davi Date: Sat, 18 Jul 2026 15:14:53 -0300 Subject: [PATCH] fix(db): dedupe bulk-imported proxies by full credential tuple (#7594) (#7644) --- src/lib/db/proxies.ts | 19 +++- .../unit/proxy-bulk-import-dedup-7594.test.ts | 105 ++++++++++++++++++ 2 files changed, 119 insertions(+), 5 deletions(-) create mode 100644 tests/unit/proxy-bulk-import-dedup-7594.test.ts diff --git a/src/lib/db/proxies.ts b/src/lib/db/proxies.ts index a32b6b59d6..d0742dfd17 100755 --- a/src/lib/db/proxies.ts +++ b/src/lib/db/proxies.ts @@ -296,9 +296,14 @@ export async function createProxy(payload: ProxyPayload) { } /** - * Upsert a proxy by host+port. - * If a proxy with the same host and port already exists, update it. - * Otherwise, create a new one. Used by the bulk import feature. + * Upsert a proxy by its credential tuple (host+port+username+password). + * If a proxy with the same host, port, username AND password already exists, + * update it. Otherwise, create a new one. Used by the bulk import feature. + * + * #7594: host+port alone is NOT a stable identity. Rotating residential/gateway + * proxies route every credential through one shared host:port, so keying only on + * host+port collapsed distinct-credential imports onto the first existing row + * (the same entry got "updated" N times instead of N entries being created). */ export async function upsertProxy(payload: ProxyPayload): Promise<{ proxy: ProxyRegistryRecord | null; @@ -307,10 +312,14 @@ export async function upsertProxy(payload: ProxyPayload): Promise<{ const db = getDbInstance(); const host = (payload.host || "").trim(); const port = Number(payload.port); + const username = (payload.username || "").trim(); + const password = (payload.password || "").trim(); const existing = db - .prepare("SELECT id FROM proxy_registry WHERE host = ? AND port = ? LIMIT 1") - .get(host, port) as { id?: string } | undefined; + .prepare( + "SELECT id FROM proxy_registry WHERE host = ? AND port = ? AND username = ? AND password = ? LIMIT 1" + ) + .get(host, port, username, password) as { id?: string } | undefined; if (existing?.id) { const updated = await updateProxy(existing.id, payload); diff --git a/tests/unit/proxy-bulk-import-dedup-7594.test.ts b/tests/unit/proxy-bulk-import-dedup-7594.test.ts new file mode 100644 index 0000000000..1ae2c0497b --- /dev/null +++ b/tests/unit/proxy-bulk-import-dedup-7594.test.ts @@ -0,0 +1,105 @@ +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"; + +// Regression test for #7594: bulk-importing proxies that share the same +// host:port but differ in username/password must create DISTINCT entries, not +// repeatedly update the first existing row. Rotating residential/gateway proxies +// commonly route every credential through one host:port, so host+port alone is +// not a stable identity — the credential tuple is. + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-proxy-dedup-7594-")); +process.env.DATA_DIR = TEST_DATA_DIR; + +const core = await import("../../src/lib/db/core.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) { + const code = (error as NodeJS.ErrnoException)?.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 }); +}); + +test("upsertProxy creates distinct entries for same host:port with different credentials (#7594)", async () => { + const base = { name: "Gateway", type: "http" as const, host: "gw.proxy.local", port: 8080 }; + + const first = await proxiesDb.upsertProxy({ ...base, username: "user-1", password: "pass-1" }); + const second = await proxiesDb.upsertProxy({ ...base, username: "user-2", password: "pass-2" }); + const third = await proxiesDb.upsertProxy({ ...base, username: "user-3", password: "pass-3" }); + + assert.equal(first.action, "created"); + assert.equal(second.action, "created"); + assert.equal(third.action, "created"); + assert.notEqual(first.proxy?.id, second.proxy?.id); + assert.notEqual(second.proxy?.id, third.proxy?.id); + + const listed = await proxiesDb.listProxies({ includeSecrets: true }); + assert.equal(listed.length, 3); + assert.deepEqual( + listed.map((p) => p.username).sort(), + ["user-1", "user-2", "user-3"] + ); +}); + +test("upsertProxy still updates when the full credential tuple matches (#7594)", async () => { + const payload = { + name: "Gateway", + type: "http" as const, + host: "gw.proxy.local", + port: 8080, + username: "user-1", + password: "pass-1", + }; + + const created = await proxiesDb.upsertProxy(payload); + const again = await proxiesDb.upsertProxy({ ...payload, name: "Gateway Renamed" }); + + assert.equal(created.action, "created"); + assert.equal(again.action, "updated"); + assert.equal(again.proxy?.id, created.proxy?.id); + + const listed = await proxiesDb.listProxies({ includeSecrets: true }); + assert.equal(listed.length, 1); + assert.equal(listed[0].name, "Gateway Renamed"); +}); + +test("upsertProxy treats auth-less proxies on same host:port as the same entry (#7594)", async () => { + const base = { name: "Open Gateway", type: "http" as const, host: "open.proxy.local", port: 3128 }; + + const first = await proxiesDb.upsertProxy(base); + const second = await proxiesDb.upsertProxy({ ...base, name: "Open Gateway 2" }); + + assert.equal(first.action, "created"); + assert.equal(second.action, "updated"); + assert.equal(first.proxy?.id, second.proxy?.id); + + const listed = await proxiesDb.listProxies({ includeSecrets: true }); + assert.equal(listed.length, 1); +});