diff --git a/docs/runtime/sql.mdx b/docs/runtime/sql.mdx index 7335d527fbdb..f4a4a662104a 100644 --- a/docs/runtime/sql.mdx +++ b/docs/runtime/sql.mdx @@ -588,6 +588,10 @@ Without a connection URL, Bun checks these individual parameters: | `PGDATABASE` | - | username | Database name | | `PGSSLMODE` | - | `disable` | SSL mode (`disable`, `allow`, `prefer`, `require`, `verify-ca`, `verify-full`) | +`PGSSLMODE` also applies when a connection URL is set. A URL from `TLS_POSTGRES_DATABASE_URL` or `TLS_DATABASE_URL` uses at least the `require` SSL mode. `PGSSLMODE=verify-ca` and `PGSSLMODE=verify-full` raise that mode and verify the server certificate. For a certificate from a private CA, pass the CA in the `tls` option: see [Custom CA Certificates](#custom-ca-certificates). + +An `sslmode`, `ssl` or `tls` parameter in the URL takes priority over the variable name and over `PGSSLMODE`. An explicit `tls` or `ssl` option takes priority over all three. + ### SQLite Environment Variables You can configure SQLite connections with `DATABASE_URL` when it contains a SQLite-compatible URL: diff --git a/src/js/internal/sql/shared.ts b/src/js/internal/sql/shared.ts index f73adbf19ad5..30ff5e8cd08e 100644 --- a/src/js/internal/sql/shared.ts +++ b/src/js/internal/sql/shared.ts @@ -1861,10 +1861,14 @@ function parseOptions( // The rest of this function is logic specific to postgres/mysql/mariadb (they have the same options object) let sslMode: SSLMode = sslModeFromConnectionDetails || SSLMode.disable; - if (sslMode === SSLMode.disable) { + if (adapter === "postgres") { // libpq honours PGSSLMODE as the default; a URL ?sslmode= below overrides it. - const envSslMode = adapter === "postgres" ? env.PG_SSLMODE || env.PGSSLMODE : undefined; - if (envSslMode) sslMode = normalizeSSLMode(envSslMode); + const envSslMode = env.PG_SSLMODE || env.PGSSLMODE; + if (envSslMode) { + // A TLS_* URL variable is a floor of `require`: PGSSLMODE raises the mode, it does not lower it. + const envMode = normalizeSSLMode(envSslMode); + if (envMode > sslMode) sslMode = envMode; + } } let url = _url; diff --git a/test/js/sql/adapter-env-var-precedence.test.ts b/test/js/sql/adapter-env-var-precedence.test.ts index 92661231639e..3f56e8de6068 100644 --- a/test/js/sql/adapter-env-var-precedence.test.ts +++ b/test/js/sql/adapter-env-var-precedence.test.ts @@ -424,6 +424,98 @@ describe("SQL adapter environment variable precedence", () => { expect(options.options.sslMode).toBe(2); expect(options.options.tls).toBeTypeOf("object"); }); + + // The TLS_ variable name asks for at least `require`. It is a floor for PGSSLMODE, not a ceiling. + describe.each(["TLS_DATABASE_URL", "TLS_POSTGRES_DATABASE_URL"])("next to a URL from %s", urlVariable => { + const url = "postgres://user@host:5432/db"; + + test.each([ + ["PGSSLMODE", "disable", 2], + ["PGSSLMODE", "allow", 2], + ["PGSSLMODE", "prefer", 2], + ["PGSSLMODE", "require", 2], + ["PGSSLMODE", "verify-ca", 3], + ["PGSSLMODE", "verify-full", 4], + ["PG_SSLMODE", "prefer", 2], + ["PG_SSLMODE", "verify-ca", 3], + ["PG_SSLMODE", "verify-full", 4], + ])("%s=%s selects sslMode %d", (modeVariable, mode, expected) => { + process.env[urlVariable] = url; + process.env[modeVariable] = mode; + + expect(new SQL().options).toMatchObject({ + adapter: "postgres", + hostname: "host", + sslMode: expected, + tls: { serverName: "host" }, + }); + }); + + test.each([ + ["a ?ssl=true in the URL", "?ssl=true", {}], + ["an explicit tls: true", "", { tls: true }], + ["an explicit tls: {}", "", { tls: {} }], + ["an explicit adapter", "", { adapter: "postgres" }], + ] as const)("PGSSLMODE=verify-full applies with %s", (_, query, options) => { + process.env[urlVariable] = url + query; + process.env.PGSSLMODE = "verify-full"; + + expect(new SQL(options).options).toMatchObject({ sslMode: 4, tls: { serverName: "host" } }); + }); + + test.each([ + ["?sslmode=disable", 0], + ["?sslmode=prefer", 1], + ["?sslmode=require", 2], + ["?sslmode=verify-ca", 3], + ["?ssl=false", 0], + ])("a URL %s overrides PGSSLMODE=verify-full", (query, expected) => { + process.env[urlVariable] = url + query; + process.env.PGSSLMODE = "verify-full"; + + expect(new SQL().options.sslMode).toBe(expected); + }); + + test.each([ + ["tls: false", { tls: false }, 0], + ["ssl: 'prefer'", { ssl: "prefer" }, 1], + ] as const)("an explicit %s overrides PGSSLMODE=verify-full", (_, options, expected) => { + process.env[urlVariable] = url; + process.env.PGSSLMODE = "verify-full"; + + expect(new SQL(options).options.sslMode).toBe(expected); + }); + + test("PGSSLMODE=verify-ca is kept when the options give a CA", () => { + process.env[urlVariable] = url; + process.env.PGSSLMODE = "verify-ca"; + + // verify-ca already checks the chain, so the CA does not raise the mode to verify-full. + expect(new SQL({ tls: { ca: "x" } }).options.sslMode).toBe(3); + }); + + test("an invalid PGSSLMODE throws", () => { + process.env[urlVariable] = url; + process.env.PGSSLMODE = "bogus"; + + expect(() => new SQL()).toThrow( + expect.objectContaining({ code: "ERR_INVALID_ARG_VALUE", message: expect.stringContaining("sslmode") }), + ); + }); + }); + + test.each([ + ["TLS_MYSQL_DATABASE_URL", "mysql://user@host:3306/db", "mysql"], + ["TLS_MARIADB_DATABASE_URL", "mariadb://user@host:3306/db", "mariadb"], + ["TLS_DATABASE_URL", "mysql://user@host:3306/db", "mysql"], + ])("PGSSLMODE is not read next to %s=%s", (urlVariable, url, adapter) => { + process.env[urlVariable] = url; + + for (const mode of ["verify-full", "bogus"]) { + process.env.PGSSLMODE = mode; + expect(new SQL().options).toMatchObject({ adapter, sslMode: 2 }); + } + }); }); describe("TLS settings from the connection URL query string", () => { diff --git a/test/js/sql/postgres-pgsslmode-env.test.ts b/test/js/sql/postgres-pgsslmode-env.test.ts index 14d2079011e3..c2bf1b8d89a4 100644 --- a/test/js/sql/postgres-pgsslmode-env.test.ts +++ b/test/js/sql/postgres-pgsslmode-env.test.ts @@ -5,7 +5,9 @@ // Buffer.alloc frame construction here. import { expect, test } from "bun:test"; -import { bunEnv, bunExe } from "harness"; +import { bunEnv, bunExe, tls as selfSignedTls } from "harness"; +import type net from "node:net"; +import tls from "node:tls"; import { listeningServer, pgAuthenticationOk, pgReadyForQuery, pgSSLResponse } from "./wire-frames"; // Bun.SQL picks up PGHOST/PGPORT/PGUSER/PGPASSWORD/PGDATABASE from the @@ -22,7 +24,7 @@ const fixture = /* js */ ` const sql = new SQL({ max: 1, connectionTimeout: 5 }); try { await sql.connect(); - console.log("CONNECTED_PLAINTEXT"); + console.log("CONNECTED"); } catch (e) { console.log("ERROR:" + (e?.code ?? e?.message ?? String(e))); } finally { @@ -55,11 +57,59 @@ async function plaintextOnlyServer() { }); } +async function selfSignedTlsServer() { + // A mock and not the postgres_tls container: the tests need what a real server + // cannot report, which is whether a StartupMessage arrived and over what. + // + // Answers SSLRequest with 'S' and upgrades to TLS with the harness certificate: + // self-signed, so no trust store accepts it, with a SAN for 127.0.0.1. Answers a + // StartupMessage, over TLS or in plaintext, with AuthenticationOk + ReadyForQuery. + const ready = Buffer.concat([pgAuthenticationOk(), pgReadyForQuery("I")]); + const startups: ("tls" | "plaintext")[] = []; + const acceptStartup = (socket: net.Socket, via: "tls" | "plaintext", buffered = Buffer.alloc(0)) => { + const onData = (chunk: Buffer) => { + buffered = Buffer.concat([buffered, chunk]); + if (buffered.length >= 8 && buffered.readInt32BE(4) === 196608) { + startups.push(via); + socket.write(ready); + buffered = Buffer.alloc(0); + } + }; + socket.on("data", onData); + onData(Buffer.alloc(0)); + }; + const { server, port } = await listeningServer(rawSocket => { + rawSocket.on("error", () => {}); + let first = Buffer.alloc(0); + const onFirstBytes = (chunk: Buffer) => { + first = Buffer.concat([first, chunk]); + if (first.length < 8) return; + rawSocket.removeListener("data", onFirstBytes); + if (first.readInt32BE(0) !== 8 || first.readInt32BE(4) !== 80877103) { + // Not an SSLRequest: the client skipped TLS. + acceptStartup(rawSocket, "plaintext", first); + return; + } + rawSocket.pause(); + // Bytes past the 8-byte SSLRequest are the start of the ClientHello. + if (first.length > 8) rawSocket.unshift(first.subarray(8)); + rawSocket.write(pgSSLResponse("S")); + const socket = new tls.TLSSocket(rawSocket, { isServer: true, key: selfSignedTls.key, cert: selfSignedTls.cert }); + socket.on("error", () => {}); + acceptStartup(socket, "tls"); + }; + rawSocket.on("data", onFirstBytes); + }); + return { server, port, startups }; +} + function pgEnv(port: number, extra: Record = {}) { const env: Record = { ...bunEnv }; for (const key of Object.keys(env)) { if (/^(PG|PG_|POSTGRES_|DATABASE_|TLS_|MYSQL|MARIADB|SQLITE)/.test(key)) delete env[key]; } + // `0` in the runner's shell turns the certificate check off for a verify-* mode. + delete env.NODE_TLS_REJECT_UNAUTHORIZED; env.PGHOST = "127.0.0.1"; env.PGPORT = String(port); env.PGUSER = "u"; @@ -115,9 +165,61 @@ test.concurrent("URL ?sslmode=disable overrides PGSSLMODE=require", async () => }); const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); expect(stderr).toBe(""); - expect(stdout.trim()).toBe("CONNECTED_PLAINTEXT"); + expect(stdout.trim()).toBe("CONNECTED"); expect(exitCode).toBe(0); } finally { await new Promise(r => server.close(() => r())); } }); + +/** Runs the fixture against a fresh self-signed TLS server, with the connection URL in `urlVariable`. */ +async function connectToSelfSignedServer(urlVariable: string, query: string, extra: Record) { + const { server, port, startups } = await selfSignedTlsServer(); + let output: [string, string, number]; + try { + await using proc = Bun.spawn({ + cmd: [bunExe(), "-e", fixture], + env: pgEnv(port, { [urlVariable]: `postgres://u:pw@127.0.0.1:${port}/db${query}`, ...extra }), + stderr: "pipe", + }); + output = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]); + } finally { + // Resolves once every connection has ended, so `startups` is final. + await new Promise(r => server.close(() => r())); + } + const [stdout, stderr, exitCode] = output; + return { stdout: stdout.trim(), stderr, exitCode, startups }; +} + +// A URL from a TLS_* variable asks for at least `require`, which does not check +// the server certificate. PGSSLMODE=verify-ca / verify-full next to it must +// still check it: the client then stops inside the TLS handshake, before it +// sends the startup packet. A ?sslmode= in the URL overrides both. +test.concurrent.each([ + [ + "a URL from TLS_DATABASE_URL alone connects without checking the certificate", + ["TLS_DATABASE_URL", "", {}], + { stdout: "CONNECTED", startups: ["tls"] }, + ], + [ + "PGSSLMODE=verify-full next to a URL from TLS_DATABASE_URL checks the certificate", + ["TLS_DATABASE_URL", "", { PGSSLMODE: "verify-full" }], + { stdout: "ERROR:DEPTH_ZERO_SELF_SIGNED_CERT", startups: [] }, + ], + [ + "PGSSLMODE=verify-ca next to a URL from TLS_POSTGRES_DATABASE_URL checks the certificate", + ["TLS_POSTGRES_DATABASE_URL", "", { PGSSLMODE: "verify-ca" }], + { stdout: "ERROR:DEPTH_ZERO_SELF_SIGNED_CERT", startups: [] }, + ], + [ + "?sslmode=disable in a URL from TLS_DATABASE_URL overrides PGSSLMODE=verify-full", + ["TLS_DATABASE_URL", "?sslmode=disable", { PGSSLMODE: "verify-full" }], + { stdout: "CONNECTED", startups: ["plaintext"] }, + ], +] as const)("%s", async (_, [urlVariable, query, extra], expected) => { + expect(await connectToSelfSignedServer(urlVariable, query, extra)).toEqual({ + ...expected, + stderr: "", + exitCode: 0, + }); +});