From c40683431b8513b6f7b113b2960d45e0339df9b9 Mon Sep 17 00:00:00 2001 From: t8y2 <1156263951@qq.com> Date: Sat, 18 Jul 2026 00:10:54 +0800 Subject: [PATCH] fix(postgres): align SSL mode behavior --- .../connection/ConnectionDialog.vue | 23 +-- .../__tests__/postgresTlsMode.spec.ts | 17 +++ .../src/lib/connection/postgresTlsMode.ts | 18 +++ crates/dbx-core/src/models/connection.rs | 31 ++-- packages/node-core/src/database.ts | 97 +++++++++++- packages/node-core/tests/postgres-ssl.test.ts | 140 ++++++++++++++++++ packages/node-core/tests/postgres-url.test.ts | 9 ++ 7 files changed, 298 insertions(+), 37 deletions(-) create mode 100644 apps/desktop/src/lib/connection/__tests__/postgresTlsMode.spec.ts create mode 100644 apps/desktop/src/lib/connection/postgresTlsMode.ts create mode 100644 packages/node-core/tests/postgres-ssl.test.ts diff --git a/apps/desktop/src/components/connection/ConnectionDialog.vue b/apps/desktop/src/components/connection/ConnectionDialog.vue index 7d073a0380..5867a018b0 100644 --- a/apps/desktop/src/components/connection/ConnectionDialog.vue +++ b/apps/desktop/src/components/connection/ConnectionDialog.vue @@ -45,6 +45,7 @@ import { JDBCX_DEFAULT_URL, JDBCX_DRIVER_PROFILE, JDBCX_JDBC_DRIVER_CLASS, ensur import { SQLITE_DATABASE_FILE_EXTENSIONS } from "@/lib/database/databaseFileDetection"; import { connectionAttemptOriginalErrorMessage, connectionAttemptTimeoutMessage, connectionAttemptTimeoutMs } from "@/lib/connection/connectionAttemptTimeout"; import { appendConnectionErrorHints } from "@/lib/connection/connectionErrorHints"; +import { postgresTlsModeForForm } from "@/lib/connection/postgresTlsMode"; import { normalizeKafkaBootstrapServers } from "@/lib/connection/kafkaBootstrapServers"; import { detectMqUiAuthKind, isMqAuthKindAllowedForSystem, type MqUiAuthKind } from "@/lib/connection/mqAuth"; import { driverInstallProgressPercent, type DriverInstallProgress } from "@/lib/connection/driverInstallProgressUi"; @@ -2046,11 +2047,7 @@ const mysqlClientKeyPath = computed({ const nativePostgresTlsDatabaseTypes = new Set(["postgres", "redshift", "gaussdb", "kwdb", "opengauss"]); const supportsPostgresTlsOptions = computed(() => nativePostgresTlsDatabaseTypes.has(form.value.db_type)); const postgresTlsMode = computed({ - get: () => { - const value = normalizePostgresSslMode(getUrlParam(form.value.url_params, "sslmode")); - if (value) return value; - return form.value.ssl ? "require" : "disable"; - }, + get: () => postgresTlsModeForForm(getUrlParam(form.value.url_params, "sslmode"), form.value.ssl), set: (value: string) => { form.value.ssl = value !== "disable"; form.value.url_params = setUrlParam(form.value.url_params, "sslmode", value); @@ -2922,22 +2919,6 @@ function applyMysqlTlsMode(params: string | undefined, mode: string): string { return setUrlParam(next, "verify_identity", "true"); } -function normalizePostgresSslMode(value: string): string { - switch (value.trim().toLowerCase()) { - case "disable": - case "prefer": - case "require": - case "verify-ca": - case "verify-full": - return value.trim().toLowerCase(); - case "verify_identity": - case "verify-identity": - return "verify-full"; - default: - return ""; - } -} - function normalizeRedisSentinelNodes(value: string): string { return normalizeRedisNodeList(value); } diff --git a/apps/desktop/src/lib/connection/__tests__/postgresTlsMode.spec.ts b/apps/desktop/src/lib/connection/__tests__/postgresTlsMode.spec.ts new file mode 100644 index 0000000000..ca47a39824 --- /dev/null +++ b/apps/desktop/src/lib/connection/__tests__/postgresTlsMode.spec.ts @@ -0,0 +1,17 @@ +import { describe, expect, it } from "vitest"; +import { postgresTlsModeForForm } from "@/lib/connection/postgresTlsMode"; + +describe("postgresTlsModeForForm", () => { + it("uses disable when legacy connections have no explicit mode", () => { + expect(postgresTlsModeForForm(undefined, false)).toBe("disable"); + }); + + it("keeps the legacy TLS toggle mapped to require", () => { + expect(postgresTlsModeForForm(undefined, true)).toBe("require"); + }); + + it("honors explicit modes and aliases", () => { + expect(postgresTlsModeForForm("disable", false)).toBe("disable"); + expect(postgresTlsModeForForm("verify_identity", false)).toBe("verify-full"); + }); +}); diff --git a/apps/desktop/src/lib/connection/postgresTlsMode.ts b/apps/desktop/src/lib/connection/postgresTlsMode.ts new file mode 100644 index 0000000000..b4f7a10c9e --- /dev/null +++ b/apps/desktop/src/lib/connection/postgresTlsMode.ts @@ -0,0 +1,18 @@ +export type PostgresTlsMode = "disable" | "prefer" | "require" | "verify-ca" | "verify-full"; + +export function postgresTlsModeForForm(value: string | undefined, ssl: boolean | undefined): PostgresTlsMode { + switch ((value || "").trim().toLowerCase()) { + case "disable": + case "prefer": + case "require": + case "verify-ca": + case "verify-full": + return value!.trim().toLowerCase() as PostgresTlsMode; + case "verify_identity": + case "verify-identity": + return "verify-full"; + default: + // DBX and DBeaver expose TLS as opt-in; an absent mode must remain plaintext. + return ssl ? "require" : "disable"; + } +} diff --git a/crates/dbx-core/src/models/connection.rs b/crates/dbx-core/src/models/connection.rs index 423d08108b..4de945f0b9 100644 --- a/crates/dbx-core/src/models/connection.rs +++ b/crates/dbx-core/src/models/connection.rs @@ -1694,8 +1694,9 @@ fn normalize_postgres_url_params(value: &str, force_tls: bool) -> String { } if connection_options.is_empty() { - if force_tls && !parts.iter().any(|part| url_param_key_is(part, "sslmode")) { - parts.insert(0, "sslmode=require".to_string()); + if !parts.iter().any(|part| url_param_key_is(part, "sslmode")) { + // TLS is opt-in in the connection form; avoid tokio-postgres' implicit prefer mode. + parts.insert(0, if force_tls { "sslmode=require" } else { "sslmode=disable" }.to_string()); } return parts.join("&"); } @@ -1722,8 +1723,8 @@ fn normalize_postgres_url_params(value: &str, force_tls: bool) -> String { parts.push(format!("options={}", encode_url_part(&combined))); } - if force_tls && !parts.iter().any(|part| url_param_key_is(part, "sslmode")) { - parts.insert(0, "sslmode=require".to_string()); + if !parts.iter().any(|part| url_param_key_is(part, "sslmode")) { + parts.insert(0, if force_tls { "sslmode=require" } else { "sslmode=disable" }.to_string()); } parts.join("&") @@ -2655,6 +2656,14 @@ mod tests { assert_eq!(config.connection_url(), "postgres://postgres:secret@10.1.2.3:2883/test?sslmode=disable"); } + #[test] + fn postgres_url_disables_tls_by_default() { + let mut config = mysql_config("postgres", "secret", Some("test")); + config.db_type = DatabaseType::Postgres; + + assert_eq!(config.connection_url(), "postgres://postgres:secret@10.1.2.3:2883/test?sslmode=disable"); + } + #[test] fn postgres_tls_switch_adds_require_sslmode() { let mut config = mysql_config("postgres", "secret", Some("test")); @@ -2686,7 +2695,7 @@ mod tests { assert_eq!( config.connection_url(), - "postgres://postgres:secret@10.1.2.3:2883/test?options=%2Dc%20search%5Fpath%3Dpublic" + "postgres://postgres:secret@10.1.2.3:2883/test?sslmode=disable&options=%2Dc%20search%5Fpath%3Dpublic" ); let pg_config = tokio_postgres::Config::from_str(&config.connection_url()).unwrap(); assert_eq!(pg_config.get_options(), Some("-c search_path=public")); @@ -2700,7 +2709,7 @@ mod tests { assert_eq!( config.connection_url(), - "postgres://postgres:secret@10.1.2.3:2883/test?options=%2Dc%20search%5Fpath%3Dapp" + "postgres://postgres:secret@10.1.2.3:2883/test?sslmode=disable&options=%2Dc%20search%5Fpath%3Dapp" ); let pg_config = tokio_postgres::Config::from_str(&config.connection_url()).unwrap(); assert_eq!(pg_config.get_options(), Some("-c search_path=app")); @@ -2734,7 +2743,7 @@ mod tests { assert_eq!( config.connection_url(), - "postgres://postgres:secret@10.1.2.3:2883/test?options=%2Dc%20statement%5Ftimeout%3D5000%20%2Dc%20TimeZone%3DUTC" + "postgres://postgres:secret@10.1.2.3:2883/test?sslmode=disable&options=%2Dc%20statement%5Ftimeout%3D5000%20%2Dc%20TimeZone%3DUTC" ); } @@ -2746,7 +2755,7 @@ mod tests { assert_eq!( config.connection_url(), - "postgres://postgres:secret@10.1.2.3:2883/test?options=-c%20TimeZone%3DUTC" + "postgres://postgres:secret@10.1.2.3:2883/test?sslmode=disable&options=-c%20TimeZone%3DUTC" ); } @@ -2755,7 +2764,7 @@ mod tests { let mut config = mysql_config("root", "secret", None); config.db_type = DatabaseType::Postgres; - assert_eq!(config.connection_url(), "postgres://root:secret@10.1.2.3:2883/postgres"); + assert_eq!(config.connection_url(), "postgres://root:secret@10.1.2.3:2883/postgres?sslmode=disable"); } #[test] @@ -2763,7 +2772,7 @@ mod tests { let mut config = mysql_config("root", "secret", Some("")); config.db_type = DatabaseType::Postgres; - assert_eq!(config.connection_url(), "postgres://root:secret@10.1.2.3:2883/postgres"); + assert_eq!(config.connection_url(), "postgres://root:secret@10.1.2.3:2883/postgres?sslmode=disable"); } #[test] @@ -2771,7 +2780,7 @@ mod tests { let mut config = mysql_config("awsuser", "secret", Some("")); config.db_type = DatabaseType::Redshift; - assert_eq!(config.connection_url(), "postgres://awsuser:secret@10.1.2.3:2883/dev"); + assert_eq!(config.connection_url(), "postgres://awsuser:secret@10.1.2.3:2883/dev?sslmode=disable"); } #[test] diff --git a/packages/node-core/src/database.ts b/packages/node-core/src/database.ts index d18b9ec5bf..8387371f5c 100644 --- a/packages/node-core/src/database.ts +++ b/packages/node-core/src/database.ts @@ -77,6 +77,8 @@ interface PoolEntry { timer: ReturnType; } +type PostgresSslMode = "disable" | "prefer" | "require" | "verify-ca" | "verify-full"; + interface RqliteResult { columns?: string[]; values?: unknown[][]; @@ -137,7 +139,7 @@ export async function closeDatabaseResources(): Promise { ); } -async function getPgPool(config: ConnectionConfig): Promise { +async function getPgPool(config: ConnectionConfig, sslModeOverride?: PostgresSslMode): Promise { const key = poolKey(config); const existing = pools.get(key); if (existing?.type === "pg") { @@ -147,8 +149,12 @@ async function getPgPool(config: ConnectionConfig): Promise { const pg = await import("pg"); const endpoint = await connectionEndpoint(config); + const sslMode = sslModeOverride ?? postgresSslMode(config); const pool = new pg.default.Pool({ - connectionString: buildConnectionUrl(config, endpoint), + // pg-connection-string lets URL SSL parameters override the explicit ssl + // object. Keep all TLS policy in one place so DBX modes cannot conflict. + connectionString: withoutPostgresSslUrlParams(buildConnectionUrl(config, endpoint)), + ssl: await postgresSslOptions(config, sslMode), max: 3, idleTimeoutMillis: 30_000, connectionTimeoutMillis: 10_000, @@ -160,6 +166,69 @@ async function getPgPool(config: ConnectionConfig): Promise { return pool; } +function postgresSslMode(config: ConnectionConfig): PostgresSslMode { + const normalized = normalizePostgresUrlParams(config.url_params || "", config.ssl); + for (const part of normalized.split("&")) { + if (!urlParamKeyIs(part, "sslmode")) continue; + const [, rawValue] = splitUrlParam(part); + const value = decodeUrlParamPart(rawValue).toLowerCase(); + if (value === "disable" || value === "prefer" || value === "require" || value === "verify-ca" || value === "verify-full") { + return value; + } + } + // TLS is opt-in in the DBX connection form; only an explicit prefer mode may downgrade. + return config.ssl ? "require" : "disable"; +} + +function postgresSslFilePaths(config: ConnectionConfig): { ca?: string; cert?: string; key?: string } { + const paths: { ca?: string; cert?: string; key?: string } = { + ca: config.ca_cert_path?.trim() || undefined, + cert: config.client_cert_path?.trim() || undefined, + key: config.client_key_path?.trim() || undefined, + }; + const normalized = normalizePostgresUrlParams(config.url_params || "", config.ssl); + for (const part of normalized.split("&")) { + const [rawKey, rawValue] = splitUrlParam(part); + const key = decodeUrlParamPart(rawKey).toLowerCase(); + const value = decodeUrlParamPart(rawValue).trim(); + if (!value) continue; + if (key === "sslrootcert") paths.ca = value; + else if (key === "sslcert") paths.cert = value; + else if (key === "sslkey") paths.key = value; + } + return paths; +} + +async function postgresSslOptions(config: ConnectionConfig, mode: PostgresSslMode): Promise { + if (mode === "disable") return false; + + const paths = postgresSslFilePaths(config); + const ssl: Exclude = {}; + if (paths.ca) ssl.ca = await readFile(paths.ca); + if (paths.cert) ssl.cert = await readFile(paths.cert); + if (paths.key) ssl.key = await readFile(paths.key); + + if (mode === "prefer" || mode === "require") { + ssl.rejectUnauthorized = false; + } else if (mode === "verify-ca") { + ssl.checkServerIdentity = () => undefined; + } + return ssl; +} + +function withoutPostgresSslUrlParams(connectionString: string): string { + const url = new URL(connectionString); + const sslKeys = new Set(["ssl", "sslmode", "ssl-mode", "sslcert", "sslkey", "sslrootcert", "uselibpqcompat"]); + for (const key of [...url.searchParams.keys()]) { + if (sslKeys.has(key.toLowerCase())) url.searchParams.delete(key); + } + return url.toString(); +} + +function postgresServerRejectedSsl(error: unknown): boolean { + return error instanceof Error && error.message === "The server does not support SSL connections"; +} + async function getMysqlPool(config: ConnectionConfig): Promise { const key = poolKey(config); const existing = pools.get(key); @@ -265,13 +334,14 @@ function normalizePostgresUrlParams(value: string, forceTls: boolean): string { if (decoded) searchPath = decoded; continue; } - if (lowerKey === "ssl-mode") { + if (lowerKey === "ssl-mode" || lowerKey === "sslmode") { const value = decodeUrlParamPart(rawValue).toLowerCase().replaceAll("_", "-"); if (value === "require" || value === "required") parts.push("sslmode=require"); else if (value === "prefer" || value === "preferred") parts.push("sslmode=prefer"); else if (value === "disable" || value === "disabled") parts.push("sslmode=disable"); else if (value === "verify-ca") parts.push("sslmode=verify-ca"); else if (value === "verify-full" || value === "verify-identity") parts.push("sslmode=verify-full"); + else if (lowerKey === "sslmode") parts.push(part); continue; } if (lowerKey === "charset" || lowerKey === "require_ssl" || lowerKey === "verify_ca" || lowerKey === "verify_identity") { @@ -776,11 +846,28 @@ async function queryWithRetry(config: ConnectionConfig, fn: () => Promise { + const sslMode = postgresSslMode(config); + let usePlaintextFallback = false; return queryWithRetry( config, async () => { - const pool = await getPgPool(config); - const result = await pool.query(sql, params); + const pool = await getPgPool(config, usePlaintextFallback ? "disable" : undefined); + let result: import("pg").QueryResult; + try { + result = await pool.query(sql, params); + } catch (error) { + if (sslMode !== "prefer" || usePlaintextFallback || !postgresServerRejectedSsl(error)) throw error; + + // Prefer may downgrade only when PostgreSQL rejects the SSLRequest + // itself. Certificate, authentication, and pg_hba failures stay fatal. + usePlaintextFallback = true; + const key = poolKey(config); + const entry = pools.get(key); + // A concurrent query may already have replaced the rejected TLS pool. + // Never evict that newer plaintext pool from a late TLS failure. + if (entry?.pool === pool) evictPool(key, entry); + result = await (await getPgPool(config, "disable")).query(sql, params); + } const rows = (result.rows || []).slice(0, resolveMaxRows(options)); return { columns: result.fields?.map((f) => f.name) ?? [], rows, row_count: rows.length }; }, diff --git a/packages/node-core/tests/postgres-ssl.test.ts b/packages/node-core/tests/postgres-ssl.test.ts new file mode 100644 index 0000000000..0c9dce440e --- /dev/null +++ b/packages/node-core/tests/postgres-ssl.test.ts @@ -0,0 +1,140 @@ +import assert from "node:assert/strict"; +import { afterEach, test, vi } from "vitest"; +import { mkdtemp, rm, writeFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import type { ConnectionConfig } from "../src/connections.js"; + +const pgMocks = vi.hoisted(() => ({ + poolConfigs: [] as Array>, + query: vi.fn(), + end: vi.fn().mockResolvedValue(undefined), + on: vi.fn(), +})); + +vi.mock("pg", () => ({ + default: { + Pool: vi.fn(function MockPool(config: Record) { + pgMocks.poolConfigs.push(config); + return { + query: (sql: string, params?: unknown[]) => pgMocks.query(config, sql, params), + end: pgMocks.end, + on: pgMocks.on, + }; + }), + }, +})); + +import { closeDatabaseResources, executeQuery } from "../src/database.js"; + +function postgresConfig(overrides: Partial = {}): ConnectionConfig { + return { + id: `pg-ssl-${Math.random()}`, + name: "pg-ssl", + db_type: "postgres", + host: "pg.example.com", + port: 5432, + username: "postgres", + password: "secret", + database: "app", + ssl: false, + ...overrides, + }; +} + +function successfulQuery() { + return { rows: [{ ok: true }], fields: [{ name: "ok" }] }; +} + +afterEach(async () => { + await closeDatabaseResources(); + pgMocks.poolConfigs.length = 0; + pgMocks.query.mockReset(); + pgMocks.end.mockClear(); + pgMocks.on.mockClear(); +}); + +test("explicit PostgreSQL prefer mode tries TLS and falls back only when SSL is unsupported", async () => { + pgMocks.query.mockImplementation((config: { ssl?: unknown }) => { + if (config.ssl === false) return Promise.resolve(successfulQuery()); + return Promise.reject(new Error("The server does not support SSL connections")); + }); + + const result = await executeQuery(postgresConfig({ url_params: "sslmode=prefer" }), "select true as ok"); + + assert.deepEqual(result.rows, [{ ok: true }]); + assert.equal(pgMocks.poolConfigs.length, 2); + assert.deepEqual(pgMocks.poolConfigs[0]?.ssl, { rejectUnauthorized: false }); + assert.equal(pgMocks.poolConfigs[1]?.ssl, false); + assert.equal(pgMocks.query.mock.calls.length, 2); +}); + +test("explicit prefer does not downgrade on authentication or certificate errors", async () => { + pgMocks.query.mockRejectedValue(new Error('no pg_hba.conf entry for host "127.0.0.1", no encryption')); + + await assert.rejects(() => executeQuery(postgresConfig({ url_params: "sslmode=prefer" }), "select 1"), /no pg_hba\.conf entry/); + + assert.equal(pgMocks.poolConfigs.length, 1); + assert.deepEqual(pgMocks.poolConfigs[0]?.ssl, { rejectUnauthorized: false }); +}); + +test("implicit PostgreSQL SSL mode remains disabled", async () => { + pgMocks.query.mockResolvedValue(successfulQuery()); + + await executeQuery(postgresConfig(), "select 1"); + + assert.equal(pgMocks.poolConfigs.length, 1); + assert.equal(pgMocks.poolConfigs[0]?.ssl, false); +}); + +test("explicit disable uses plaintext and never retries TLS negotiation failures", async () => { + pgMocks.query.mockRejectedValue(new Error("The server does not support SSL connections")); + + await assert.rejects(() => executeQuery(postgresConfig({ url_params: "sslmode=disable" }), "select 1"), /does not support SSL/); + + assert.equal(pgMocks.poolConfigs.length, 1); + assert.equal(pgMocks.poolConfigs[0]?.ssl, false); + assert.doesNotMatch(String(pgMocks.poolConfigs[0]?.connectionString), /sslmode=/); +}); + +test("require and verification modes never downgrade", async () => { + for (const mode of ["require", "verify-ca", "verify-full", "verify_identity"] as const) { + pgMocks.query.mockRejectedValueOnce(new Error("The server does not support SSL connections")); + + await assert.rejects(() => executeQuery(postgresConfig({ url_params: `sslmode=${mode}` }), "select 1"), /does not support SSL/); + } + + assert.equal(pgMocks.poolConfigs.length, 4); + assert.deepEqual(pgMocks.poolConfigs[0]?.ssl, { rejectUnauthorized: false }); + assert.equal(typeof (pgMocks.poolConfigs[1]?.ssl as { checkServerIdentity?: unknown }).checkServerIdentity, "function"); + assert.deepEqual(pgMocks.poolConfigs[2]?.ssl, {}); + assert.deepEqual(pgMocks.poolConfigs[3]?.ssl, {}); +}); + +test("PostgreSQL SSL files are loaded into the ssl object and removed from the connection string", async () => { + const directory = await mkdtemp(join(tmpdir(), "dbx-pg-ssl-")); + const caPath = join(directory, "ca.pem"); + const certPath = join(directory, "client.pem"); + const keyPath = join(directory, "client.key"); + await Promise.all([writeFile(caPath, "ca"), writeFile(certPath, "cert"), writeFile(keyPath, "key")]); + pgMocks.query.mockResolvedValue(successfulQuery()); + + try { + await executeQuery( + postgresConfig({ + url_params: `sslmode=verify-full&sslrootcert=${encodeURIComponent(caPath)}&sslcert=${encodeURIComponent(certPath)}&sslkey=${encodeURIComponent(keyPath)}&application_name=dbx`, + }), + "select 1", + ); + } finally { + await rm(directory, { recursive: true, force: true }); + } + + const poolConfig = pgMocks.poolConfigs[0]; + assert.equal(poolConfig?.connectionString, "postgres://postgres:secret@pg.example.com:5432/app?application_name=dbx"); + assert.deepEqual(poolConfig?.ssl, { + ca: Buffer.from("ca"), + cert: Buffer.from("cert"), + key: Buffer.from("key"), + }); +}); diff --git a/packages/node-core/tests/postgres-url.test.ts b/packages/node-core/tests/postgres-url.test.ts index 6cd4f48a9d..91fb82d509 100644 --- a/packages/node-core/tests/postgres-url.test.ts +++ b/packages/node-core/tests/postgres-url.test.ts @@ -42,6 +42,15 @@ test("postgres MCP URL maps ssl-mode to sslmode", () => { assert.equal(url, "postgres://postgres:secret@pg.example.com:5432/app?sslmode=require"); }); +test("postgres MCP URL normalizes direct sslmode aliases", () => { + const url = buildConnectionUrl(postgresConfig({ url_params: "sslmode=verify_identity" }), { + host: "pg.example.com", + port: 5432, + }); + + assert.equal(url, "postgres://postgres:secret@pg.example.com:5432/app?sslmode=verify-full"); +}); + test("postgres MCP URL drops MySQL-style TLS params", () => { const url = buildConnectionUrl( postgresConfig({