From 3e64b22e56f6ce5e824be7ce5e066cabf83f11ff Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Fri, 17 Jul 2026 19:42:25 -0300 Subject: [PATCH] fix(api): import route guards (provider,name) collisions instead of silent overwrite (#6836, #2587) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /api/providers/import route called createProviderConnection directly, which upserts-by-name for apikey auth — so re-imported or same-batch duplicate rows silently overwrote existing credentials while still reporting 'created'. Mirror the sibling /bulk route's resolveBulkNameCollisions guard. --- src/app/api/providers/import/route.ts | 51 ++++++++++++- .../api/providers-import-route-6836.test.ts | 71 +++++++++++++++++++ 2 files changed, 119 insertions(+), 3 deletions(-) diff --git a/src/app/api/providers/import/route.ts b/src/app/api/providers/import/route.ts index b096dd76de..c1272b07af 100644 --- a/src/app/api/providers/import/route.ts +++ b/src/app/api/providers/import/route.ts @@ -4,9 +4,15 @@ import { getProviderAuditTarget, summarizeProviderConnectionForAudit, } from "@/lib/compliance/providerAudit"; -import { createProviderConnection, getProviderNodeById, isCloudEnabled } from "@/models"; +import { + createProviderConnection, + getProviderConnections, + getProviderNodeById, + isCloudEnabled, +} from "@/models"; import { isAnthropicCompatibleProvider, isOpenAICompatibleProvider } from "@/shared/constants/providers"; import { getConsistentMachineId } from "@/shared/utils/machineId"; +import { resolveBulkNameCollisions } from "@/shared/utils/bulkApiKeyParser"; import { syncToCloud } from "@/lib/cloudSync"; import { bulkImportProviderSchema } from "@/shared/validation/schemas"; import { isValidationFailure, validateBody } from "@/shared/validation/helpers"; @@ -109,6 +115,44 @@ async function importOneEntry( return { created: safe }; } +/** + * #2587 / #6836 — mirrors the guard in POST /api/providers/bulk: createProviderConnection + * upserts apikey connections BY NAME, so an imported row whose (provider, name) collides + * with an already-saved connection — or with an earlier row in the SAME import batch — + * would silently REPLACE that connection's apiKey/priority instead of inserting a new + * one, while the response still reported it as a fresh "created" success. Unlike /bulk + * (single provider per request), one import batch can span many DIFFERENT providers, so + * collisions are resolved per-provider: existing connection names are fetched once per + * distinct provider in the batch, then `resolveBulkNameCollisions` gap-fills a free + * " " suffix for every entry so each one reaches createProviderConnection as a + * genuine insert. + */ +async function resolveImportNameCollisions(entries: ImportEntry[]): Promise { + const indicesByProvider = new Map(); + entries.forEach((entry, index) => { + const indices = indicesByProvider.get(entry.provider) || []; + indices.push(index); + indicesByProvider.set(entry.provider, indices); + }); + + const resolved: ImportEntry[] = [...entries]; + for (const [provider, indices] of indicesByProvider) { + const existingConnections = await getProviderConnections({ provider, authType: "apikey" }); + const existingNames = existingConnections + .map((c) => (typeof c.name === "string" ? c.name : null)) + .filter((n): n is string => !!n); + + const providerEntries = indices.map((i) => ({ name: entries[i].name })); + const resolvedProviderEntries = resolveBulkNameCollisions(providerEntries, existingNames); + + indices.forEach((originalIndex, i) => { + resolved[originalIndex] = { ...entries[originalIndex], name: resolvedProviderEntries[i].name }; + }); + } + + return resolved; +} + async function syncToCloudIfEnabled() { try { const cloudEnabled = await isCloudEnabled(); @@ -143,12 +187,13 @@ export async function POST(request: Request) { } const { entries, validateKeys } = validation.data; + const resolvedEntries = await resolveImportNameCollisions(entries); const created: Array> = []; const errors: Array<{ index: number; name: string; provider: string; message: string }> = []; - for (let i = 0; i < entries.length; i++) { - const entry = entries[i]; + for (let i = 0; i < resolvedEntries.length; i++) { + const entry = resolvedEntries[i]; try { const result = await importOneEntry(entry, !!validateKeys); if ("error" in result) { diff --git a/tests/unit/api/providers-import-route-6836.test.ts b/tests/unit/api/providers-import-route-6836.test.ts index f8286328a1..86ceb2185a 100644 --- a/tests/unit/api/providers-import-route-6836.test.ts +++ b/tests/unit/api/providers-import-route-6836.test.ts @@ -119,6 +119,77 @@ test("providers import route: partial-failure — unresolvable compatible node f assert.ok(!JSON.stringify(body).includes(" at /")); }); +test("providers import route: same-batch (provider,name) collision does not overwrite the first row (#2587-class data loss)", async () => { + await resetStorage(); + const response = await postImport({ + entries: [ + { provider: "openai", name: "Prod OpenAI", apiKey: "sk-openai-first" }, + { provider: "openai", name: "Prod OpenAI", apiKey: "sk-openai-second" }, + ], + }); + assert.equal(response.status, 200); + const body = (await response.json()) as ImportRouteResponse; + assert.equal(body.total, 2); + assert.equal(body.success, 2, "both rows must be created — the second must not silently upsert into the first"); + assert.equal(body.failed, 0); + assert.equal(body.created.length, 2); + + const providersDb = await import("../../../src/lib/db/providers.ts"); + const connections = (await providersDb.getProviderConnections({ + provider: "openai", + })) as Array<{ id: string; name?: string | null; apiKey?: string }>; + assert.equal(connections.length, 2, "the collision must produce TWO distinct connections, never one"); + + const first = connections.find((c) => c.apiKey === "sk-openai-first"); + const second = connections.find((c) => c.apiKey === "sk-openai-second"); + assert.ok(first, "the first row's apiKey must survive unmodified"); + assert.ok(second, "the second row must be a genuine insert, not a silent overwrite"); + assert.notEqual(first!.id, second!.id, "the two rows must be distinct connections"); + assert.notEqual( + first!.name, + second!.name, + "the colliding row must be disambiguated with a distinct name, not silently share the first row's name" + ); +}); + +test("providers import route: re-importing an existing (provider,name) does not overwrite the saved connection", async () => { + await resetStorage(); + const providersDb = await import("../../../src/lib/db/providers.ts"); + const existing = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "Prod OpenAI", + apiKey: "sk-existing", + priority: 3, + testStatus: "unavailable", + lastError: "429 rate limited", + }); + assert.ok(existing); + + const response = await postImport({ + entries: [{ provider: "openai", name: "Prod OpenAI", apiKey: "sk-reimported" }], + }); + assert.equal(response.status, 200); + const body = (await response.json()) as ImportRouteResponse; + assert.equal(body.success, 1); + + const connections = (await providersDb.getProviderConnections({ + provider: "openai", + })) as Array<{ id: string; name?: string | null; apiKey?: string; testStatus?: string; lastError?: string }>; + assert.equal(connections.length, 2, "re-import must APPEND a new connection, not replace the existing one"); + + const survivor = connections.find((c) => c.id === existing!.id); + assert.ok(survivor, "the pre-existing connection must still exist, unreplaced"); + assert.equal(survivor!.apiKey, "sk-existing", "existing apiKey must not be overwritten by the re-import"); + assert.equal(survivor!.testStatus, "unavailable", "existing testStatus must survive the re-import"); + assert.equal(survivor!.lastError, "429 rate limited", "existing lastError must survive the re-import"); + + const imported = connections.find((c) => c.id !== existing!.id); + assert.ok(imported, "the newly imported row must exist as a distinct connection"); + assert.equal(imported!.apiKey, "sk-reimported"); + assert.notEqual(imported!.name, "Prod OpenAI", "the re-imported row must be disambiguated, not collide on name"); +}); + test("providers import route applies a per-entry baseUrl override for compatible providers", async () => { await resetStorage(); // openai-compatible providers require a registered node; without one the row fails