diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index 02a39f74c5..fae797e357 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -19,7 +19,12 @@ import { } from "@omniroute/open-sse/services/apiKeyRotator.ts"; import { invalidateReasoningRoutingRuleCache } from "./reasoningRoutingRules"; import { normalizeProviderSpecificData } from "@/lib/providers/requestDefaults"; -import { bumpProxyConfigGeneration } from "./settings"; +import { bumpProxyConfigGeneration, getSettings } from "./settings"; +import { + getStoredManagementPassword, + isBcryptHash, + verifyManagementPassword, +} from "@/lib/auth/managementPassword"; import { webSessionCredentialKey, parseProviderSpecificData } from "./webSessionDedup"; import { pickCodexConnectionForUser } from "@/lib/oauth/utils/codexConnectionSelection"; import { reconcileCodexUsageHistory } from "./providers/usageIdentityReconciliation"; @@ -40,6 +45,55 @@ type JsonRecord = Record; const CONNECTION_CREDENTIAL_FIELDS = ["apiKey", "accessToken", "refreshToken", "idToken"] as const; +/** Thrown when a write would store the dashboard login password as a provider credential. */ +export class ManagementPasswordAsCredentialError extends Error { + readonly code = "MANAGEMENT_PASSWORD_AS_CREDENTIAL" as const; + + constructor() { + super( + "That value is the dashboard login password, not a provider API key. Storing it would " + + "send it upstream on every request routed through this connection." + ); + this.name = "ManagementPasswordAsCredentialError"; + } +} + +/** + * Refuse to store the dashboard login password as a connection API key. + * + * A browser that autofills the management password into the API-key field + * produces a connection whose credential authenticates against nothing, and + * every request routed through it comes back 401. Rejecting it in the form + * would not be enough: the same autofill fires again while an operator is + * repairing the connection by hand, so the refusal has to sit on the write + * path that all of those forms funnel into. + * + * Only an actual match blocks the write. A settings row that cannot be read, + * or a bcrypt call that throws, logs and allows -- a guard against one specific + * operator mistake must not become a way to lock out every connection write. + */ +async function assertApiKeyIsNotManagementPassword(apiKey: unknown): Promise { + const candidate = typeof apiKey === "string" ? apiKey.trim() : ""; + if (!candidate) return; + + try { + const settings = (await getSettings()) as JsonRecord; + const stored = getStoredManagementPassword(settings); + // Only a stored bcrypt hash is comparable. A fresh install that has never + // bootstrapped a password has nothing to collide with. + if (!isBcryptHash(stored)) return; + if (!(await verifyManagementPassword(candidate, stored))) return; + } catch (err) { + console.warn( + "[Providers] could not check the credential against the dashboard password:", + err instanceof Error ? err.message : String(err) + ); + return; + } + + throw new ManagementPasswordAsCredentialError(); +} + interface StatementLike { all: (...params: unknown[]) => TRow[]; get: (...params: unknown[]) => TRow | undefined; @@ -282,6 +336,7 @@ function findExistingCookieConnection( } export async function createProviderConnection(data: JsonRecord) { + await assertApiKeyIsNotManagementPassword(data.apiKey); const db = getDbInstance() as unknown as DbLike; const now = new Date().toISOString(); const normalizedProviderSpecificData = normalizeProviderSpecificData( @@ -730,6 +785,12 @@ export async function updateProviderConnection(id: string, data: JsonRecord) { const existing = db.prepare("SELECT * FROM provider_connections WHERE id = ?").get(id); if (!existing) return null; + // The incoming value only. A connection that already holds the password has + // to stay editable, or an operator cannot repair the one this guard exists + // to prevent -- and re-checking the merged value would spend a bcrypt round + // on every unrelated field edit. + await assertApiKeyIsNotManagementPassword(data.apiKey); + const merged: JsonRecord = { ...toRecord(rowToCamel(existing)), ...data, diff --git a/tests/unit/reject-management-password-as-apikey.test.ts b/tests/unit/reject-management-password-as-apikey.test.ts new file mode 100644 index 0000000000..2d6c241be0 --- /dev/null +++ b/tests/unit/reject-management-password-as-apikey.test.ts @@ -0,0 +1,181 @@ +/** + * The dashboard login password must never be storable as a provider API key. + * + * A browser autofilling the management password into the API-key field created + * connections that 401 every request routed through them, and the same autofill + * fired again while the connection was being repaired by hand. These assert the + * refusal sits on the write path rather than in any one form. + */ + +import { after, beforeEach, describe, it } 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-apikey-guard-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.DISABLE_SQLITE_AUTO_BACKUP = "true"; + +const core = await import("../../src/lib/db/core.ts"); +const providersDb = await import("../../src/lib/db/providers.ts"); +const settingsDb = await import("../../src/lib/db/settings.ts"); +const mgmt = await import("../../src/lib/auth/managementPassword.ts"); + +const DASHBOARD_PASSWORD = "correct-horse-battery-staple"; + +/** getStoredManagementPassword reads `settings.password`, holding a bcrypt hash. */ +async function storeDashboardPassword(plaintext: string) { + await settingsDb.updateSettings({ password: await mgmt.hashManagementPassword(plaintext) }); +} + +beforeEach(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); +}); + +after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +describe("management password as a provider credential", () => { + it("create is refused when the apiKey is the dashboard password", async () => { + await storeDashboardPassword(DASHBOARD_PASSWORD); + + await assert.rejects( + () => + providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "autofilled", + apiKey: DASHBOARD_PASSWORD, + isActive: true, + }), + (err: Error) => { + assert.equal(err.name, "ManagementPasswordAsCredentialError"); + assert.ok( + !err.message.includes(DASHBOARD_PASSWORD), + "the refusal must not echo the password back" + ); + return true; + } + ); + + const rows = await providersDb.getProviderConnections({ provider: "openai" }); + assert.equal(rows.length, 0, "nothing may be persisted when the guard fires"); + }); + + it("create is refused on surrounding whitespace, which a paste carries", async () => { + await storeDashboardPassword(DASHBOARD_PASSWORD); + + await assert.rejects( + () => + providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "pasted", + apiKey: ` ${DASHBOARD_PASSWORD} `, + isActive: true, + }), + /dashboard login password/ + ); + }); + + it("a real API key is stored normally", async () => { + await storeDashboardPassword(DASHBOARD_PASSWORD); + + const conn = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "genuine", + apiKey: "sk-a-real-provider-key", + isActive: true, + }); + assert.ok(conn?.id); + }); + + it("update is refused too, which is where the repair attempt gets re-infected", async () => { + await storeDashboardPassword(DASHBOARD_PASSWORD); + + const conn = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "genuine", + apiKey: "sk-a-real-provider-key", + isActive: true, + }); + assert.ok(conn?.id); + + await assert.rejects( + () => providersDb.updateProviderConnection(conn.id as string, { apiKey: DASHBOARD_PASSWORD }), + /dashboard login password/ + ); + + const stored = await providersDb.getProviderConnectionById(conn.id as string); + assert.equal(stored?.apiKey, "sk-a-real-provider-key", "the good key must survive the refusal"); + }); + + it("an update that does not carry an apiKey is untouched by the guard", async () => { + await storeDashboardPassword(DASHBOARD_PASSWORD); + + const conn = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "before", + apiKey: "sk-a-real-provider-key", + isActive: true, + }); + assert.ok(conn?.id); + + const updated = await providersDb.updateProviderConnection(conn.id as string, { + name: "after", + }); + assert.equal(updated?.name, "after"); + }); + + it("a connection already holding the password stays editable, so it can be repaired", async () => { + // Seed the bad state the way the incident produced it: the password was + // stored before the guard existed. Write it with no dashboard password + // configured, then configure one. + const conn = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "poisoned", + apiKey: DASHBOARD_PASSWORD, + isActive: true, + }); + assert.ok(conn?.id, "no dashboard password configured yet, so the write goes through"); + await storeDashboardPassword(DASHBOARD_PASSWORD); + + const repaired = await providersDb.updateProviderConnection(conn.id as string, { + apiKey: "sk-the-actual-key", + }); + assert.equal(repaired?.apiKey, "sk-the-actual-key"); + }); + + it("no dashboard password configured means nothing to collide with", async () => { + const conn = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "fresh install", + apiKey: DASHBOARD_PASSWORD, + isActive: true, + }); + assert.ok(conn?.id); + }); + + it("an OAuth connection carrying no apiKey never reaches the bcrypt round", async () => { + await storeDashboardPassword(DASHBOARD_PASSWORD); + + const conn = await providersDb.createProviderConnection({ + provider: "claude", + authType: "oauth", + name: "oauth", + accessToken: DASHBOARD_PASSWORD, + isActive: true, + }); + assert.ok(conn?.id, "the guard covers apiKey only; OAuth tokens are not operator-typed"); + }); +});