From 6e71553797b1bc8ab36ae384c944f4401fb90b9b Mon Sep 17 00:00:00 2001 From: Paijo <14921983+oyi77@users.noreply.github.com> Date: Sat, 18 Jul 2026 21:33:32 +0700 Subject: [PATCH] perf(db): project columns + composite index in getProviderConnections (#6918) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * perf(db): project columns + composite index in getProviderConnections - Add param to avoid (scans ~200MB/query) - Add WHERE clause support (was silently ignored) - Add composite index on (auth_type, is_active, refresh_token) - Update health check caller to request only needed columns - Test: authType filter, column projection, default full fetch * fix(db): dedupe authType filter, allowlist columns projection in getProviderConnections The branch was rebased on top of #6946 (already merged, same author, same authType-filter fix), leaving a duplicate `if (filter.authType)` block in getProviderConnections. Harmless at runtime (SQLite tolerates the repeated named param) but dead code — remove the newer duplicate, keep the one already merged via #6946. The `columns` projection param is interpolated directly into the SQL SELECT clause via `.join(", ")` with no validation. No current caller passes untrusted input, but it's a live SQL-injection footgun for whichever future caller wires it up: reproduced a working exfiltration via a single-statement subquery column name (no semicolon/stacked-query needed, so better-sqlite3's single-statement restriction doesn't help) that leaked an unrelated connection's api_key through the response. Add an allowlist validated against the real provider_connections schema (core.ts's SCHEMA_SQL) — rejects any non-listed column, and re-quotes the reserved "group" keyword so it stays usable. Verified the fix blocks the exact reproduced exfiltration. Add a regression test asserting invalid/injection-shaped column names are rejected, a mixed valid+invalid list still rejects (fail-closed, not a silent partial projection), and the legitimate "group" column still round-trips correctly when requested. Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --------- Co-authored-by: oyi77 Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> --- src/lib/db/core.ts | 4 +- src/lib/db/providers.ts | 88 ++++++++++++++++++++++---- tests/unit/db-providers-crud.test.ts | 95 ++++++++++++++++++++++++++++ 3 files changed, 174 insertions(+), 13 deletions(-) diff --git a/src/lib/db/core.ts b/src/lib/db/core.ts index 8207b09a8a..69bd022ca5 100644 --- a/src/lib/db/core.ts +++ b/src/lib/db/core.ts @@ -248,6 +248,7 @@ const SCHEMA_SQL = ` CREATE INDEX IF NOT EXISTS idx_pc_provider ON provider_connections(provider); CREATE INDEX IF NOT EXISTS idx_pc_active ON provider_connections(is_active); CREATE INDEX IF NOT EXISTS idx_pc_priority ON provider_connections(provider, priority); + CREATE INDEX IF NOT EXISTS idx_pc_auth_active_refresh ON provider_connections(auth_type, is_active, refresh_token); CREATE TABLE IF NOT EXISTS provider_nodes ( id TEXT PRIMARY KEY, @@ -1041,8 +1042,7 @@ export function getDbInstance(): SqliteDatabase { let hasData = false; try { const count = probe.prepare("SELECT COUNT(*) as c FROM provider_connections").get() as - | { c: number } - | undefined; + { c: number } | undefined; hasData = Boolean(count && count.c > 0); } catch { // Table might not exist at all — truly incompatible diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index 1217c860b3..e3f6e1241e 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -39,11 +39,75 @@ interface DbLike { prepare: (sql: string) => StatementLike; } +// Real column set for provider_connections (must match the CREATE TABLE in +// core.ts's SCHEMA_SQL). getProviderConnections()'s optional `columns` +// projection is interpolated directly into the SELECT clause, so every +// requested name must be validated against this allowlist before use — +// there is no current caller that passes untrusted input, but the +// projection API itself must never accept an arbitrary string. +const PROVIDER_CONNECTIONS_COLUMNS = new Set([ + "id", + "provider", + "auth_type", + "name", + "email", + "priority", + "is_active", + "access_token", + "refresh_token", + "expires_at", + "token_expires_at", + "scope", + "project_id", + "test_status", + "error_code", + "last_error", + "last_error_at", + "last_error_type", + "last_error_source", + "backoff_level", + "rate_limited_until", + "health_check_interval", + "last_health_check_at", + "last_tested", + "api_key", + "id_token", + "provider_specific_data", + "expires_in", + "display_name", + "global_priority", + "default_model", + "token_type", + "consecutive_use_count", + "rate_limit_protection", + "last_used_at", + "group", + "max_concurrent", + "proxy_enabled", + "per_key_proxy_enabled", + "quota_window_thresholds_json", + "rate_limit_overrides_json", + "created_at", + "updated_at", +]); + // ──────────────── Provider Connections ──────────────── -export async function getProviderConnections(filter: JsonRecord = {}) { +export async function getProviderConnections(filter: JsonRecord = {}, columns?: string[]) { const db = getDbInstance() as unknown as DbLike; - let sql = "SELECT * FROM provider_connections"; + let selectCols = "*"; + if (columns?.length) { + const invalidColumns = columns.filter((col) => !PROVIDER_CONNECTIONS_COLUMNS.has(col)); + if (invalidColumns.length > 0) { + throw new Error( + `getProviderConnections: invalid column(s) requested: ${invalidColumns.join(", ")}` + ); + } + // "group" is a reserved SQL keyword — the schema declares it quoted, so + // it must be re-quoted here too or the generated SELECT is a syntax error. + selectCols = columns.map((col) => (col === "group" ? `"group"` : col)).join(", "); + } + let sql = `SELECT ${selectCols} FROM provider_connections`; const conditions: string[] = []; const params: Record = {}; @@ -621,8 +685,9 @@ export async function clearConnectionErrorIfUnchanged( } ): Promise { const db = getDbInstance() as unknown as DbLike; - const result = db.prepare( - ` + const result = db + .prepare( + ` UPDATE provider_connections SET test_status = 'active', last_error = NULL, @@ -638,13 +703,14 @@ export async function clearConnectionErrorIfUnchanged( AND IFNULL(last_error_at, '') = ? AND IFNULL(rate_limited_until, '') = ? ` - ).run( - new Date().toISOString(), - id, - expected.testStatus ?? "", - expected.lastErrorAt ?? "", - expected.rateLimitedUntil ?? "" - ); + ) + .run( + new Date().toISOString(), + id, + expected.testStatus ?? "", + expected.lastErrorAt ?? "", + expected.rateLimitedUntil ?? "" + ); const applied = (result.changes ?? 0) > 0; if (applied) { backupDbFile("pre-write"); diff --git a/tests/unit/db-providers-crud.test.ts b/tests/unit/db-providers-crud.test.ts index 9c005fc179..cde22ba705 100644 --- a/tests/unit/db-providers-crud.test.ts +++ b/tests/unit/db-providers-crud.test.ts @@ -384,3 +384,98 @@ test("quota helpers zero stale windows and format countdowns", () => { assert.match(providersDb.formatResetCountdown(future), /1m \d+s/); assert.equal(providersDb.formatResetCountdown(past), null); }); +test("getProviderConnections supports authType filter and column projection", async () => { + const oauth = await providersDb.createProviderConnection({ + provider: "openai", + authType: "oauth", + name: "OAuth Conn", + email: "user@example.com", + refreshToken: "rt_abc123", + isActive: true, + }); + const apiKey = await providersDb.createProviderConnection({ + provider: "openai", + authType: "apikey", + name: "API Key Conn", + apiKey: "sk-xyz", + isActive: true, + }); + + // authType filter + const oauthConns = await providersDb.getProviderConnections({ authType: "oauth" }); + assert.equal(oauthConns.length, 1); + assert.equal(oauthConns[0].id, oauth.id); + + const apiKeyConns = await providersDb.getProviderConnections({ authType: "apikey" }); + assert.equal(apiKeyConns.length, 1); + assert.equal(apiKeyConns[0].id, apiKey.id); + + // authType + isActive filter + const activeOAuth = await providersDb.getProviderConnections({ + authType: "oauth", + isActive: true, + }); + assert.equal(activeOAuth.length, 1); + + // Column projection: only requested columns returned + const projected = await providersDb.getProviderConnections({ authType: "oauth" }, [ + "id", + "provider", + "name", + ]); + assert.equal(projected.length, 1); + const keys = Object.keys(projected[0]); + // id, provider, name each appear in camelCase + assert.ok(keys.includes("id")); + assert.ok(keys.includes("provider")); + assert.ok(keys.includes("name")); + // decryptConnectionFields always adds undefined keys via explicit spread, + // so `in` checks can't distinguish "not projected" from "undefined value." + // Check value semantics instead. + assert.strictEqual(projected[0].refreshToken, undefined); + assert.strictEqual(projected[0].authType, undefined); + + // Default (no columns param) returns all fields + const full = await providersDb.getProviderConnections({ authType: "oauth" }); + const fullKeys = Object.keys(full[0]); + assert.ok(fullKeys.length > keys.length); + assert.ok("refreshToken" in full[0]); + assert.ok("authType" in full[0]); +}); + +test("getProviderConnections rejects column names outside the real provider_connections schema", async () => { + // The `columns` array is interpolated directly into the SQL SELECT clause, + // so it must be validated against an allowlist before use — otherwise it's + // a SQL-injection footgun for whichever future caller wires it to + // untrusted input. A single bogus column should reject the whole call. + await assert.rejects( + () => providersDb.getProviderConnections({}, ["not_a_real_column"]), + /invalid column/i + ); + + // A mix of valid + invalid columns must still reject (fail-closed, not a + // silent partial projection). + await assert.rejects( + () => providersDb.getProviderConnections({}, ["id", "provider; DROP TABLE provider_connections; --"]), + /invalid column/i + ); + + // The reserved SQL keyword "group" is a legitimate, allowlisted column and + // must still work (quoted internally so it doesn't collide with the SQL + // GROUP keyword). + await providersDb.createProviderConnection({ + provider: "openai", + authType: "oauth", + name: "Group Column Conn", + email: "group-col@example.com", + refreshToken: "rt_group_col", + isActive: true, + group: "team-a", + }); + const withGroup = await providersDb.getProviderConnections({ authType: "oauth" }, [ + "id", + "group", + ]); + assert.equal(withGroup.length, 1); + assert.equal(withGroup[0].group, "team-a"); +});