mirror of
https://github.com/diegosouzapw/OmniRoute.git
synced 2026-07-26 09:52:11 +03:00
perf(db): project columns + composite index in getProviderConnections (#6918)
* 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 <oyi77@users.noreply.github.com> Co-authored-by: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -39,11 +39,75 @@ interface DbLike {
|
||||
prepare: <TRow = unknown>(sql: string) => StatementLike<TRow>;
|
||||
}
|
||||
|
||||
// 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<string, unknown> = {};
|
||||
|
||||
@@ -621,8 +685,9 @@ export async function clearConnectionErrorIfUnchanged(
|
||||
}
|
||||
): Promise<boolean> {
|
||||
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");
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user