Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/fixes/7707-proxy-password-rotation.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **fix(db):** bulk proxy imports now update an existing proxy when only its password changes instead of creating a duplicate ([#7707](https://github.com/diegosouzapw/OmniRoute/pull/7707)) — thanks @floze-the-genius
38 changes: 27 additions & 11 deletions src/lib/db/proxies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -296,14 +296,17 @@ export async function createProxy(payload: ProxyPayload) {
}

/**
* 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.
* Upsert a proxy by its identity tuple (host+port+username).
* If a proxy with the same host, port, and username 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).
*
* #7703: password is mutable and must not be part of the identity key. Including
* it caused password-only credential rotations to create duplicate entries.
*/
export async function upsertProxy(payload: ProxyPayload): Promise<{
proxy: ProxyRegistryRecord | null;
Expand All @@ -313,13 +316,10 @@ export async function upsertProxy(payload: ProxyPayload): Promise<{
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 = ? AND username = ? AND password = ? LIMIT 1"
)
.get(host, port, username, password) as { id?: string } | undefined;
.prepare("SELECT id FROM proxy_registry WHERE host = ? AND port = ? AND username = ? LIMIT 1")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Update all duplicate proxy identities on rotation

For users who already imported a rotated password before this fix, proxy_registry can already contain multiple rows with the same host, port, and username because there is no uniqueness constraint. This new lookup selects only one arbitrary row with LIMIT 1, so a later password rotation updates whichever duplicate SQLite returns while the other row(s)—including any assignments that point at them—keep stale passwords and remain duplicated. Please handle the full duplicate set when matching by host/port/username, or merge/update them deterministically.

Useful? React with 👍 / 👎.

.get(host, port, username) as { id?: string } | undefined;

if (existing?.id) {
const updated = await updateProxy(existing.id, payload);
Expand Down Expand Up @@ -701,13 +701,23 @@ function getOrCreateRotationRow(
db: ReturnType<typeof getDbInstance>,
normalizedScope: string,
rotationScopeId: string
): { strategy: ProxyRotationStrategy; cursor: number; stickyWindowMinutes: number; rotatedAt: string | null } {
): {
strategy: ProxyRotationStrategy;
cursor: number;
stickyWindowMinutes: number;
rotatedAt: string | null;
} {
const row = db
.prepare(
"SELECT strategy, cursor, sticky_window_minutes, rotated_at FROM proxy_scope_rotation WHERE scope = ? AND scope_id IS ?"
)
.get(normalizedScope, rotationScopeId) as
| { strategy?: string; cursor?: number; sticky_window_minutes?: number; rotated_at?: string | null }
| {
strategy?: string;
cursor?: number;
sticky_window_minutes?: number;
rotated_at?: string | null;
}
| undefined;

if (row) {
Expand Down Expand Up @@ -766,7 +776,13 @@ function pickFromCandidates<T>(
cursor = state.cursor + 1;
db.prepare(
"UPDATE proxy_scope_rotation SET cursor = ?, rotated_at = ?, updated_at = ? WHERE scope = ? AND scope_id IS ?"
).run(cursor, new Date().toISOString(), new Date().toISOString(), normalizedScope, rotationScopeId);
).run(
cursor,
new Date().toISOString(),
new Date().toISOString(),
normalizedScope,
rotationScopeId
);
}
const idx = ((cursor % candidates.length) + candidates.length) % candidates.length;
return candidates[idx];
Expand Down
34 changes: 29 additions & 5 deletions tests/unit/proxy-bulk-import-dedup-7594.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,7 @@ test("upsertProxy creates distinct entries for same host:port with different cre

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"]
);
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 () => {
Expand All @@ -90,8 +87,35 @@ test("upsertProxy still updates when the full credential tuple matches (#7594)",
assert.equal(listed[0].name, "Gateway Renamed");
});

test("upsertProxy updates an existing proxy when only its password changes (#7703)", async () => {
const payload = {
name: "Rotating Gateway",
type: "http" as const,
host: "rotate.proxy.local",
port: 8080,
username: "stable-user",
password: "pass-a",
};

const created = await proxiesDb.upsertProxy(payload);
const rotated = await proxiesDb.upsertProxy({ ...payload, password: "pass-b" });

assert.equal(created.action, "created");
assert.equal(rotated.action, "updated");
assert.equal(rotated.proxy?.id, created.proxy?.id);

const listed = await proxiesDb.listProxies({ includeSecrets: true });
assert.equal(listed.length, 1);
assert.equal(listed[0].password, "pass-b");
});

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 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" });
Expand Down