diff --git a/.changeset/cli-tls-handling.md b/.changeset/cli-tls-handling.md new file mode 100644 index 000000000..ae221d33d --- /dev/null +++ b/.changeset/cli-tls-handling.md @@ -0,0 +1,10 @@ +--- +'stash': minor +--- + +The CLI now handles database TLS properly, so the discoverable fix for a certificate failure is never `NODE_TLS_REJECT_UNAUTHORIZED=0`. + +- Every CLI database connection honours `sslmode` and `sslrootcert` from the connection string — and `PGSSLMODE` / `PGSSLROOTCERT` from the environment when the URL carries no TLS parameters (URL wins; unlike raw node-postgres, `PGSSLROOTCERT` is actually consumed): `verify-full` (and `require`/`verify-ca`/`prefer`, kept as full verification — node-postgres's current behaviour) verifies the server certificate; `no-verify` is honoured with a one-line stderr warning; `disable` turns TLS off. Client-certificate setups (`sslcert`/`sslkey`) pass through untouched. +- CA resolution: `sslrootcert=` (libpq semantics — sole trust anchor; `sslrootcert=system` selects the system store) → `PGSSLROOTCERT` → for `*.supabase.co`/`*.supabase.com` hosts a **bundled Supabase root CA** (appended to the system roots) → the system store. `sslmode=verify-full` against Supabase — direct hosts and the pgBouncer pooler — now verifies out of the box. +- Certificate-verification failures — shaped centrally in the connection factory, so every command surfaces them — name the host and the supported remedies in order (`sslrootcert=…`, then `sslmode=no-verify` as a last resort with the consequence spelled out), and explicitly warn against `NODE_TLS_REJECT_UNAUTHORIZED=0`, which is process-wide and would also disable verification for the connections carrying CipherStash credentials. +- The node-postgres "SSL modes … are treated as aliases for verify-full" SECURITY WARNING no longer appears on every invocation against `sslmode=require` URLs: the CLI decides the TLS config itself and hands pg a URL with the TLS params stripped (fixes the upstream-advisory passthrough). diff --git a/packages/cli/src/commands/db/install.ts b/packages/cli/src/commands/db/install.ts index 7b5fed75f..e9aafad6e 100644 --- a/packages/cli/src/commands/db/install.ts +++ b/packages/cli/src/commands/db/install.ts @@ -1,8 +1,8 @@ import { installMigrationsSchema } from '@cipherstash/migrate' import * as p from '@clack/prompts' -import pg from 'pg' import { resolveDatabaseUrl } from '@/config/database-url.js' import { findConfigFile, loadStashConfig } from '@/config/index.js' +import { createPgClient } from '@/db/client.js' import { EQLInstaller } from '@/installer/index.js' import { messages } from '@/messages.js' import { detectPackageManager, runnerCommand } from '../init/utils.js' @@ -186,7 +186,7 @@ export async function installCommand( if (supabase) reportSupabaseGrantsOutcome(installResult) s.start('Installing cs_migrations tracking schema...') - const migrationsDb = new pg.Client({ connectionString: databaseUrl }) + const migrationsDb = createPgClient(databaseUrl) try { await migrationsDb.connect() await installMigrationsSchema(migrationsDb) diff --git a/packages/cli/src/commands/db/status.ts b/packages/cli/src/commands/db/status.ts index 761016682..34ce0aece 100644 --- a/packages/cli/src/commands/db/status.ts +++ b/packages/cli/src/commands/db/status.ts @@ -1,7 +1,7 @@ import * as p from '@clack/prompts' -import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' +import { createPgClient } from '@/db/client.js' import { EQLInstaller } from '@/installer/index.js' export async function statusCommand(options: { databaseUrl?: string } = {}) { @@ -110,7 +110,7 @@ export async function statusCommand(options: { databaseUrl?: string } = {}) { s.start('Checking encrypt configuration...') - const client = new pg.Client({ connectionString: config.databaseUrl }) + const client = createPgClient(config.databaseUrl) try { await client.connect() diff --git a/packages/cli/src/commands/db/test-connection.ts b/packages/cli/src/commands/db/test-connection.ts index 879f111cf..6d3e69787 100644 --- a/packages/cli/src/commands/db/test-connection.ts +++ b/packages/cli/src/commands/db/test-connection.ts @@ -1,8 +1,8 @@ import * as p from '@clack/prompts' -import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { detectDotenvFile } from '@/config/database-url.js' import { loadStashConfig } from '@/config/index.js' +import { createPgClient, TlsVerificationError } from '@/db/client.js' import { messages } from '@/messages.js' export async function testConnectionCommand( @@ -16,7 +16,7 @@ export async function testConnectionCommand( const config = await loadStashConfig({ databaseUrlFlag: options.databaseUrl }) s.stop('Configuration loaded.') - const client = new pg.Client({ connectionString: config.databaseUrl }) + const client = createPgClient(config.databaseUrl) try { s.start('Connecting to database...') @@ -48,7 +48,12 @@ export async function testConnectionCommand( const message = error instanceof Error ? error.message : 'An unknown error occurred' - p.log.error(`Failed to connect to database: ${message}`) + if (error instanceof TlsVerificationError) { + // Shaped centrally by createPgClient — self-contained, print verbatim. + p.log.error(error.message) + } else { + p.log.error(`Failed to connect to database: ${message}`) + } console.log() p.log.info(messages.db.urlConnectionFailedHint(detectDotenvFile())) process.exit(1) diff --git a/packages/cli/src/commands/encrypt/backfill.ts b/packages/cli/src/commands/encrypt/backfill.ts index 216bd1f5b..21d73a5fc 100644 --- a/packages/cli/src/commands/encrypt/backfill.ts +++ b/packages/cli/src/commands/encrypt/backfill.ts @@ -17,6 +17,7 @@ import * as p from '@clack/prompts' import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' +import { buildPgClientConfig, explainTlsError } from '@/db/client.js' import { loadEncryptionContext, requireTable } from './context.js' /** @@ -115,8 +116,11 @@ export async function backfillCommand(options: BackfillCommandOptions) { const ctx = await loadEncryptionContext() const tableSchema = requireTable(ctx, options.table) + // Through the TLS-aware config builder like every other connection — + // pg.PoolConfig extends pg.ClientConfig, so the pool inherits the same + // sslmode/sslrootcert handling and the bundled Supabase CA. const pool = new pg.Pool({ - connectionString: stashConfig.databaseUrl, + ...buildPgClientConfig(stashConfig.databaseUrl), max: 2, }) @@ -131,7 +135,17 @@ export async function backfillCommand(options: BackfillCommandOptions) { try { process.on('SIGINT', onSignal) process.on('SIGTERM', onSignal) - db = await pool.connect() + try { + db = await pool.connect() + } catch (error) { + // A certificate-verification failure is an author-shaped, row-data-free + // diagnostic — route it through BackfillConfigError so it prints + // verbatim (the generic handler below deliberately suppresses message + // text, which would bury the remedy). + const tlsExplanation = explainTlsError(error, stashConfig.databaseUrl) + if (tlsExplanation) throw new BackfillConfigError(tlsExplanation) + throw error + } // `stash eql install` normally creates `cipherstash.cs_migrations`, but // not every integration runs it — Prisma Next installs EQL through its diff --git a/packages/cli/src/commands/encrypt/drop.ts b/packages/cli/src/commands/encrypt/drop.ts index ec7871a35..7ade0fabc 100644 --- a/packages/cli/src/commands/encrypt/drop.ts +++ b/packages/cli/src/commands/encrypt/drop.ts @@ -9,10 +9,10 @@ import { setManifestTargetPhase, } from '@cipherstash/migrate' import * as p from '@clack/prompts' -import pg from 'pg' import { detectDrizzle } from '@/commands/db/detect.js' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' +import { createPgClient } from '@/db/client.js' import { scaffoldDrizzleMigration } from './drizzle-helper.js' import { explainUnresolved, resolveColumnLifecycle } from './lib/resolve-eql.js' @@ -58,7 +58,7 @@ export async function dropCommand(options: DropCommandOptions) { p.intro(runnerCommand(detectPackageManager(), 'stash encrypt drop')) const config = await loadStashConfig() - const client = new pg.Client({ connectionString: config.databaseUrl }) + const client = createPgClient(config.databaseUrl) let exitCode = 0 try { diff --git a/packages/cli/src/commands/encrypt/plan.ts b/packages/cli/src/commands/encrypt/plan.ts index 4a3523705..d4fd3760d 100644 --- a/packages/cli/src/commands/encrypt/plan.ts +++ b/packages/cli/src/commands/encrypt/plan.ts @@ -1,8 +1,8 @@ import { latestByColumn, readManifest } from '@cipherstash/migrate' import * as p from '@clack/prompts' -import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' +import { createPgClient } from '@/db/client.js' /** * CLI handler for `stash encrypt plan`. Reads the repo manifest and the @@ -24,7 +24,7 @@ export async function planCommand() { return } - const client = new pg.Client({ connectionString: config.databaseUrl }) + const client = createPgClient(config.databaseUrl) let exitCode = 0 try { await client.connect() diff --git a/packages/cli/src/commands/encrypt/status.ts b/packages/cli/src/commands/encrypt/status.ts index 0554f74dc..8f99593c4 100644 --- a/packages/cli/src/commands/encrypt/status.ts +++ b/packages/cli/src/commands/encrypt/status.ts @@ -4,9 +4,9 @@ import { readManifest, } from '@cipherstash/migrate' import * as p from '@clack/prompts' -import pg from 'pg' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadStashConfig } from '@/config/index.js' +import { createPgClient } from '@/db/client.js' import { type EqlColumnInfo, fetchActiveEqlConfig, @@ -41,7 +41,7 @@ export async function statusCommand() { const config = await loadStashConfig() const manifest = await readManifest(process.cwd()) - const client = new pg.Client({ connectionString: config.databaseUrl }) + const client = createPgClient(config.databaseUrl) let exitCode = 0 try { diff --git a/packages/cli/src/commands/eql/applied.ts b/packages/cli/src/commands/eql/applied.ts index 01d5e497f..d4168a683 100644 --- a/packages/cli/src/commands/eql/applied.ts +++ b/packages/cli/src/commands/eql/applied.ts @@ -5,6 +5,8 @@ * `migrations.table` in drizzle.config.ts); a project that overrides them must * say so, because the probe cannot discover it — see {@link LEDGER_ABSENT}. */ +import { buildPgClientConfig } from '@/db/config.js' + export const DEFAULT_MIGRATIONS_RELATION = 'drizzle.__drizzle_migrations' /** @@ -93,7 +95,7 @@ export async function latestAppliedMillis( relation: string = DEFAULT_MIGRATIONS_RELATION, ): Promise { const { default: pg } = await import('pg') - const client = new pg.Client({ connectionString: databaseUrl }) + const client = new pg.Client(buildPgClientConfig(databaseUrl)) try { await client.connect() const result = await client.query<{ max_created_at: string | null }>( diff --git a/packages/cli/src/commands/eql/validate.ts b/packages/cli/src/commands/eql/validate.ts index 3a3cb056f..f55691fdd 100644 --- a/packages/cli/src/commands/eql/validate.ts +++ b/packages/cli/src/commands/eql/validate.ts @@ -1,10 +1,10 @@ import type { AnyV3Table } from '@cipherstash/stack/eql/v3' import type { ColumnSchema, EncryptConfig } from '@cipherstash/stack/schema' import * as p from '@clack/prompts' -import pg from 'pg' import { fetchPhysicalColumns } from '@/commands/encrypt/lib/db-readers.js' import { detectPackageManager, runnerCommand } from '@/commands/init/utils.js' import { loadEncryptSchemas, loadStashConfig } from '@/config/index.js' +import { createPgClient } from '@/db/client.js' // --------------------------------------------------------------------------- // The vocabulary @@ -875,7 +875,7 @@ async function tryReadObservedState( } const tables = [...new Set(columns.map((column) => column.table))] - const client = new pg.Client({ connectionString: databaseUrl }) + const client = createPgClient(databaseUrl) try { await client.connect() diff --git a/packages/cli/src/commands/init/lib/introspect.ts b/packages/cli/src/commands/init/lib/introspect.ts index fb2d6c3d1..37cef7049 100644 --- a/packages/cli/src/commands/init/lib/introspect.ts +++ b/packages/cli/src/commands/init/lib/introspect.ts @@ -1,5 +1,5 @@ import * as p from '@clack/prompts' -import pg from 'pg' +import { createPgClient } from '@/db/client.js' import type { ColumnDef, DataType, SchemaDef, V3Domain } from '../types.js' export interface DbColumn { @@ -81,8 +81,7 @@ export async function introspectDatabase( // this, an unreachable / firewalled database silently hangs the spinner // until the user kills the process. 10 s is generous for healthy hosts // and short enough to surface a real failure quickly. - const client = new pg.Client({ - connectionString: databaseUrl, + const client = createPgClient(databaseUrl, { connectionTimeoutMillis: 10_000, }) try { diff --git a/packages/cli/src/commands/init/lib/rollout-state.ts b/packages/cli/src/commands/init/lib/rollout-state.ts index cfcbff81f..773e6887f 100644 --- a/packages/cli/src/commands/init/lib/rollout-state.ts +++ b/packages/cli/src/commands/init/lib/rollout-state.ts @@ -1,5 +1,5 @@ import type { MigrationPhase } from '@cipherstash/migrate' -import pg from 'pg' +import { createPgClient } from '@/db/client.js' import { latestByColumnSafe } from '../../encrypt/lib/db-readers.js' /** Conservative connect timeout for rollout-state lookups: the CLI @@ -65,8 +65,7 @@ export async function detectColumnStates( ): Promise { if (columns.length === 0) return [] - const client = new pg.Client({ - connectionString: databaseUrl, + const client = createPgClient(databaseUrl, { connectionTimeoutMillis: CONNECT_TIMEOUT_MS, }) try { diff --git a/packages/cli/src/commands/status/index.ts b/packages/cli/src/commands/status/index.ts index a3449ff20..3a13f2b5b 100644 --- a/packages/cli/src/commands/status/index.ts +++ b/packages/cli/src/commands/status/index.ts @@ -6,7 +6,7 @@ import { readManifest, } from '@cipherstash/migrate' import * as p from '@clack/prompts' -import pg from 'pg' +import { createPgClient } from '@/db/client.js' import { fetchActiveEqlConfig, fetchPhysicalColumns, @@ -114,8 +114,7 @@ export async function gatherObservations( } } - const client = new pg.Client({ - connectionString: databaseUrl, + const client = createPgClient(databaseUrl, { connectionTimeoutMillis: CONNECT_TIMEOUT_MS, }) const tables = Array.from(new Set(targetColumns.map((c) => c.table))) diff --git a/packages/cli/src/db/__tests__/client-wrap.test.ts b/packages/cli/src/db/__tests__/client-wrap.test.ts new file mode 100644 index 000000000..17ca8dbe8 --- /dev/null +++ b/packages/cli/src/db/__tests__/client-wrap.test.ts @@ -0,0 +1,59 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const mockConnect = vi.fn() +const mockEnd = vi.fn() + +vi.mock('pg', () => ({ + default: { + Client: vi.fn(() => { + const client: Record = { + connect: (...args: unknown[]) => mockConnect(...args), + end: mockEnd, + } + return client + }), + }, +})) + +describe('createPgClient connect wrapping', () => { + beforeEach(() => vi.clearAllMocks()) + afterEach(() => vi.restoreAllMocks()) + + it('re-throws certificate failures as TlsVerificationError with the remedy', async () => { + mockConnect.mockRejectedValue( + Object.assign(new Error('self-signed certificate in certificate chain'), { + code: 'SELF_SIGNED_CERT_IN_CHAIN', + }), + ) + const { createPgClient, TlsVerificationError } = await import( + '../client.js' + ) + const client = createPgClient( + 'postgres://u@aws-0-us-east-1.pooler.supabase.com/postgres?sslmode=require', + ) + const failure = await client.connect().catch((error: unknown) => error) + expect(failure).toBeInstanceOf(TlsVerificationError) + expect((failure as Error).message).toContain( + 'aws-0-us-east-1.pooler.supabase.com', + ) + expect((failure as Error).message).toContain('sslrootcert=') + expect((failure as Error).message).toContain( + 'Never set NODE_TLS_REJECT_UNAUTHORIZED=0', + ) + }) + + it('passes non-TLS connect failures through untouched', async () => { + const original = new Error('password authentication failed for user "u"') + mockConnect.mockRejectedValue(original) + const { createPgClient } = await import('../client.js') + const client = createPgClient('postgres://u@h/app?sslmode=require') + await expect(client.connect()).rejects.toBe(original) + }) + + it('resolves normally when connect succeeds', async () => { + mockConnect.mockResolvedValue(undefined) + const { createPgClient } = await import('../client.js') + const client = createPgClient('postgres://u@h/app') + await expect(client.connect()).resolves.toBeUndefined() + }) +}) diff --git a/packages/cli/src/db/__tests__/config.test.ts b/packages/cli/src/db/__tests__/config.test.ts new file mode 100644 index 000000000..38b8a1dba --- /dev/null +++ b/packages/cli/src/db/__tests__/config.test.ts @@ -0,0 +1,252 @@ +import { mkdtempSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import tls from 'node:tls' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { + buildPgClientConfig, + explainTlsError, + resetNoVerifyWarningForTests, +} from '../config.js' +import { SUPABASE_ROOT_CA_PEM } from '../supabase-ca.js' + +type SslObject = { rejectUnauthorized?: boolean; ca?: string | string[] } + +function ssl(config: ReturnType): SslObject { + expect(config.ssl).toBeTypeOf('object') + return config.ssl as SslObject +} + +describe('buildPgClientConfig', () => { + beforeEach(() => { + resetNoVerifyWarningForTests() + vi.unstubAllEnvs() + }) + afterEach(() => vi.restoreAllMocks()) + + it('passes a URL with no TLS params through untouched', () => { + const url = 'postgres://user:pass@db.example.com:5432/app' + expect(buildPgClientConfig(url)).toEqual({ connectionString: url }) + }) + + it('passes non-URL connection strings through untouched', () => { + expect(buildPgClientConfig('not a url')).toEqual({ + connectionString: 'not a url', + }) + }) + + it('passes client-certificate setups through untouched', () => { + const url = + 'postgres://u@db.example.com/app?sslmode=verify-full&sslcert=c.pem&sslkey=k.pem' + expect(buildPgClientConfig(url)).toEqual({ connectionString: url }) + }) + + it('merges extra client options in every arm', () => { + const plain = buildPgClientConfig('postgres://u@h/app', { + connectionTimeoutMillis: 10_000, + }) + expect(plain.connectionTimeoutMillis).toBe(10_000) + const tlsful = buildPgClientConfig('postgres://u@h/app?sslmode=require', { + connectionTimeoutMillis: 10_000, + }) + expect(tlsful.connectionTimeoutMillis).toBe(10_000) + }) + + it('sslmode=disable turns TLS off and strips the param', () => { + const config = buildPgClientConfig( + 'postgres://u@db.example.com/app?sslmode=disable', + ) + expect(config.ssl).toBe(false) + expect(config.connectionString).not.toContain('sslmode') + }) + + it('sslmode=no-verify keeps encryption without verification, warning once on stderr', () => { + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true) + const first = buildPgClientConfig( + 'postgres://u@db.example.com/app?sslmode=no-verify', + ) + expect(ssl(first).rejectUnauthorized).toBe(false) + buildPgClientConfig('postgres://u@db.example.com/app?sslmode=no-verify') + expect(stderr).toHaveBeenCalledTimes(1) + expect(String(stderr.mock.calls[0]?.[0])).toContain('NOT authenticated') + }) + + it.each([ + 'require', + 'prefer', + 'verify-ca', + 'verify-full', + ])('sslmode=%s verifies fully and strips the param', (mode) => { + const config = buildPgClientConfig( + `postgres://u@db.example.com/app?sslmode=${mode}`, + ) + expect(ssl(config).rejectUnauthorized).toBe(true) + expect(config.connectionString).not.toContain('sslmode') + }) + + it('honours sslrootcert= as the sole trust anchor (libpq semantics)', () => { + const dir = mkdtempSync(join(tmpdir(), 'stash-ca-')) + const caPath = join(dir, 'root.pem') + writeFileSync(caPath, 'FAKE PEM CONTENT') + const config = buildPgClientConfig( + `postgres://u@db.example.com/app?sslmode=verify-full&sslrootcert=${caPath}`, + ) + expect(ssl(config).ca).toBe('FAKE PEM CONTENT') + expect(config.connectionString).not.toContain('sslrootcert') + }) + + it('sslrootcert=system selects the system trust store', () => { + const config = buildPgClientConfig( + 'postgres://u@db.example.com/app?sslmode=verify-full&sslrootcert=system', + ) + expect(ssl(config).rejectUnauthorized).toBe(true) + expect(ssl(config).ca).toBeUndefined() + }) + + it('fails loudly when the sslrootcert file cannot be read', () => { + expect(() => + buildPgClientConfig( + 'postgres://u@db.example.com/app?sslmode=verify-full&sslrootcert=/nonexistent/ca.pem', + ), + ).toThrow(/Cannot read the CA file named by sslrootcert/) + }) + + it('falls back to PGSSLROOTCERT when the URL names no CA', () => { + const dir = mkdtempSync(join(tmpdir(), 'stash-ca-')) + const caPath = join(dir, 'env.pem') + writeFileSync(caPath, 'ENV PEM') + vi.stubEnv('PGSSLROOTCERT', caPath) + const config = buildPgClientConfig( + 'postgres://u@db.example.com/app?sslmode=require', + ) + expect(ssl(config).ca).toBe('ENV PEM') + }) + + it('appends the bundled Supabase root CA to the system roots for Supabase hosts', () => { + for (const host of [ + 'db.abcdefghij.supabase.co', + 'aws-0-us-east-1.pooler.supabase.com', + ]) { + const config = buildPgClientConfig( + `postgres://u@${host}:5432/postgres?sslmode=require`, + ) + const ca = ssl(config).ca + expect(Array.isArray(ca)).toBe(true) + expect(ca).toContain(SUPABASE_ROOT_CA_PEM) + expect((ca as string[]).length).toBe(tls.rootCertificates.length + 1) + } + }) + + it('does not attach the Supabase CA to non-Supabase hosts', () => { + const config = buildPgClientConfig( + 'postgres://u@db.example.com/app?sslmode=require', + ) + expect(ssl(config).ca).toBeUndefined() + }) + + it('a bare sslrootcert with no sslmode still verifies fully', () => { + const dir = mkdtempSync(join(tmpdir(), 'stash-ca-')) + const caPath = join(dir, 'bare.pem') + writeFileSync(caPath, 'BARE PEM') + const config = buildPgClientConfig( + `postgres://u@db.example.com/app?sslrootcert=${caPath}`, + ) + expect(ssl(config).rejectUnauthorized).toBe(true) + expect(ssl(config).ca).toBe('BARE PEM') + }) +}) + +describe('PGSSLMODE environment tier', () => { + beforeEach(() => { + resetNoVerifyWarningForTests() + vi.unstubAllEnvs() + }) + + it('enables verification from PGSSLMODE=require, with CA resolution', () => { + vi.stubEnv('PGSSLMODE', 'require') + const config = buildPgClientConfig( + 'postgres://u@aws-0-us-east-1.pooler.supabase.com:5432/postgres', + ) + expect(ssl(config).rejectUnauthorized).toBe(true) + // The whole point over pg's own env handling: the CA tier runs, so the + // bundled Supabase root applies (pg ignores PGSSLROOTCERT entirely). + expect(ssl(config).ca).toContain(SUPABASE_ROOT_CA_PEM) + }) + + it('honours PGSSLROOTCERT alongside PGSSLMODE', () => { + const dir = mkdtempSync(join(tmpdir(), 'stash-ca-')) + const caPath = join(dir, 'env-tier.pem') + writeFileSync(caPath, 'ENV TIER PEM') + vi.stubEnv('PGSSLMODE', 'verify-full') + vi.stubEnv('PGSSLROOTCERT', caPath) + const config = buildPgClientConfig('postgres://u@db.example.com/app') + expect(ssl(config).rejectUnauthorized).toBe(true) + expect(ssl(config).ca).toBe('ENV TIER PEM') + }) + + it('mirrors pg for PGSSLMODE=disable and no-verify', () => { + vi.stubEnv('PGSSLMODE', 'disable') + expect(buildPgClientConfig('postgres://u@h/app').ssl).toBe(false) + vi.stubEnv('PGSSLMODE', 'no-verify') + const stderr = vi + .spyOn(process.stderr, 'write') + .mockImplementation(() => true) + expect( + ssl(buildPgClientConfig('postgres://u@h/app')).rejectUnauthorized, + ).toBe(false) + expect(stderr).toHaveBeenCalledTimes(1) + }) + + it('passes through on an unrecognised PGSSLMODE, exactly like pg', () => { + vi.stubEnv('PGSSLMODE', 'allow') + const url = 'postgres://u@h/app' + expect(buildPgClientConfig(url)).toEqual({ connectionString: url }) + }) + + it('lets URL parameters beat the environment (libpq precedence)', () => { + vi.stubEnv('PGSSLMODE', 'require') + const config = buildPgClientConfig('postgres://u@h/app?sslmode=disable') + expect(config.ssl).toBe(false) + }) +}) + +describe('explainTlsError', () => { + const url = 'postgres://u@aws-0-ap-southeast-2.pooler.supabase.com/postgres' + + it('names the host and the remedies for a self-signed chain', () => { + const explanation = explainTlsError( + Object.assign(new Error('self-signed certificate in certificate chain'), { + code: 'SELF_SIGNED_CERT_IN_CHAIN', + }), + url, + ) + expect(explanation).toContain('aws-0-ap-southeast-2.pooler.supabase.com') + expect(explanation).toContain('sslrootcert=') + expect(explanation).toContain('sslmode=no-verify') + expect(explanation).toContain('Never set NODE_TLS_REJECT_UNAUTHORIZED=0') + }) + + it('recognises hostname-mismatch failures', () => { + expect( + explainTlsError( + Object.assign( + new Error("Hostname/IP does not match certificate's altnames"), + { + code: 'ERR_TLS_CERT_ALTNAME_INVALID', + }, + ), + url, + ), + ).not.toBeNull() + }) + + it('returns null for non-TLS failures', () => { + expect( + explainTlsError(new Error('password authentication failed'), url), + ).toBeNull() + expect(explainTlsError(new Error('ECONNREFUSED'), url)).toBeNull() + expect(explainTlsError(null, url)).toBeNull() + }) +}) diff --git a/packages/cli/src/db/__tests__/no-sslmode-advisory.test.ts b/packages/cli/src/db/__tests__/no-sslmode-advisory.test.ts new file mode 100644 index 000000000..eeeffff6f --- /dev/null +++ b/packages/cli/src/db/__tests__/no-sslmode-advisory.test.ts @@ -0,0 +1,17 @@ +import pg from 'pg' +import { describe, expect, it } from 'vitest' +import { buildPgClientConfig } from '../config.js' + +describe('#822 — no upstream sslmode advisory', () => { + it('constructing a client via the factory emits no process warning', async () => { + const warnings: string[] = [] + const listener = (w: Error) => warnings.push(w.message) + process.on('warning', listener) + new pg.Client( + buildPgClientConfig('postgres://u:p@h:5432/db?sslmode=require'), + ) + await new Promise((resolve) => setTimeout(resolve, 200)) + process.off('warning', listener) + expect(warnings.filter((m) => m.includes('SECURITY WARNING'))).toEqual([]) + }) +}) diff --git a/packages/cli/src/db/__tests__/pg-construction-lint.test.ts b/packages/cli/src/db/__tests__/pg-construction-lint.test.ts new file mode 100644 index 000000000..2a2308af3 --- /dev/null +++ b/packages/cli/src/db/__tests__/pg-construction-lint.test.ts @@ -0,0 +1,60 @@ +/** + * Every database connection the CLI opens must go through the TLS-aware + * config layer in `src/db/` — otherwise it silently skips sslmode/sslrootcert + * handling, the bundled Supabase CA, and the shaped certificate errors, and + * re-opens the exact gap this layer closed (`encrypt backfill`'s `pg.Pool` + * was the fifteenth site, missed because the sweep counted `pg.Client`). + * + * Same spirit as protect-ffi's `lintWiring.test.ts`: a rule that only lives + * in review comments is a rule the sixteenth site will break. Enforced form: + * any `new pg.Client(` / `new pg.Pool(` in `src/` production code must either + * be `src/db/client.ts` (the factory itself) or pass `buildPgClientConfig` + * within the constructor call. Test files are exempt — live suites connect + * to local fixtures with URLs they build themselves. + */ + +import { readdirSync, readFileSync } from 'node:fs' +import { join, relative, sep } from 'node:path' +import { describe, expect, it } from 'vitest' + +const SRC_ROOT = join(__dirname, '..', '..') + +function sourceFiles(dir: string): string[] { + const out: string[] = [] + for (const entry of readdirSync(dir, { withFileTypes: true })) { + const full = join(dir, entry.name) + if (entry.isDirectory()) { + if (entry.name === '__tests__' || entry.name === 'node_modules') continue + out.push(...sourceFiles(full)) + } else if (entry.name.endsWith('.ts') && !entry.name.endsWith('.test.ts')) { + out.push(full) + } + } + return out +} + +describe('pg construction routes through the TLS config layer', () => { + it('no bare new pg.Client(...) / new pg.Pool(...) outside src/db/client.ts', () => { + const offenders: string[] = [] + for (const file of sourceFiles(SRC_ROOT)) { + const rel = relative(SRC_ROOT, file).split(sep).join('/') + if (rel === 'db/client.ts') continue + const content = readFileSync(file, 'utf-8') + const pattern = /new pg\.(?:Client|Pool)\(/g + for (const match of content.matchAll(pattern)) { + // The constructor's argument must involve buildPgClientConfig — + // inspect the text immediately following the call site (covers both + // `new pg.Client(buildPgClientConfig(url))` and the pool's + // `{ ...buildPgClientConfig(url), max: 2 }` spread form). + const argWindow = content.slice( + match.index, + match.index + match[0].length + 120, + ) + if (!argWindow.includes('buildPgClientConfig')) { + offenders.push(`${rel}: ${argWindow.split('\n')[0]}`) + } + } + } + expect(offenders).toEqual([]) + }) +}) diff --git a/packages/cli/src/db/client.ts b/packages/cli/src/db/client.ts new file mode 100644 index 000000000..96d028663 --- /dev/null +++ b/packages/cli/src/db/client.ts @@ -0,0 +1,51 @@ +/** + * The one place the CLI turns a database URL into a `pg.Client`. The TLS + * policy — `sslmode`/`sslrootcert` handling, the bundled Supabase root CA, + * and cert-error shaping — lives in `./config.ts`; this module just binds it + * to the driver. + */ + +import pg from 'pg' +import { + buildPgClientConfig, + explainTlsError, + TlsVerificationError, +} from './config.js' + +export { + buildPgClientConfig, + explainTlsError, + resetNoVerifyWarningForTests, + TlsVerificationError, +} from './config.js' + +/** + * Build a client whose `connect()` re-throws certificate-verification + * failures as {@link TlsVerificationError} carrying the shaped remedy — + * centrally, so every command that awaits `connect()` surfaces the + * host-specific fix without each call site knowing about TLS. Non-TLS + * failures pass through untouched. + */ +export function createPgClient( + databaseUrl: string, + extra: Omit = {}, +): pg.Client { + const client = new pg.Client(buildPgClientConfig(databaseUrl, extra)) + const originalConnect = client.connect.bind(client) + const wrappedConnect = async (): Promise => { + try { + await originalConnect() + } catch (error) { + if (error instanceof TlsVerificationError) throw error + const explanation = explainTlsError(error, databaseUrl) + if (explanation) { + throw new TlsVerificationError(explanation, { cause: error }) + } + throw error + } + } + // Every CLI call site uses the promise form; the callback overload is not + // used and the assertion narrows to the declared union. + client.connect = wrappedConnect as typeof client.connect + return client +} diff --git a/packages/cli/src/db/config.ts b/packages/cli/src/db/config.ts new file mode 100644 index 000000000..354178322 --- /dev/null +++ b/packages/cli/src/db/config.ts @@ -0,0 +1,216 @@ +/** + * TLS-aware `pg.ClientConfig` construction — the policy half of the CLI's + * database connections. Runtime-import-free on `pg` (type-only), so the + * offline-capable commands that lazy-load the driver (`eql repair`'s applied + * probe) can build a config without pulling `pg` in. The convenience factory + * lives in `./client.ts`. + * + * node-postgres does two things we can't ship as-is in a security product: + * + * 1. It treats `sslmode=prefer|require|verify-ca` as aliases for + * `verify-full` and prints a process-level SECURITY WARNING saying so on + * every invocation against such URLs (#822). We keep the verify-full + * semantics but decide them ourselves, handing pg an explicit `ssl` + * config and a URL with the TLS params stripped — same behaviour, no + * upstream advisory on our stdout. + * + * 2. It has no CA story. Managed providers (Supabase) sign their server + * certificates with a private CA, so verification fails with + * `self-signed certificate in certificate chain` and the only + * discoverable fix used to be `NODE_TLS_REJECT_UNAUTHORIZED=0` — + * process-wide, covering the connections that carry ZeroKMS credentials + * (#889). We honour `sslrootcert` (and `PGSSLROOTCERT`), bundle the + * Supabase root CA for `*.supabase.co|com` hosts, and shape cert errors + * into the supported remedies. + * + * URLs with no `sslmode`/`sslrootcert` at all — and URLs using client + * certificates (`sslcert`/`sslkey`) or the raw `ssl` param — pass through + * untouched: zero behaviour change outside the parameters we understand. + */ + +import { readFileSync } from 'node:fs' +import tls from 'node:tls' +import type pg from 'pg' +import { SUPABASE_ROOT_CA_PEM } from './supabase-ca.js' + +/** Hosts the bundled Supabase root CA applies to (db.* and pooler.*). */ +const SUPABASE_HOST_PATTERN = /\.supabase\.(?:co|com)$/i + +/** Params this module consumes; everything else stays on the URL. */ +const HANDLED_PARAMS = ['sslmode', 'sslrootcert'] as const + +/** Params that mean "hand-tuned TLS setup — don't touch the URL". */ +const PASSTHROUGH_PARAMS = ['sslcert', 'sslkey', 'sslpassword', 'ssl'] as const + +/** The PGSSLMODE values node-postgres itself recognises — mirrored exactly. */ +const ENV_SSLMODES = [ + 'disable', + 'prefer', + 'require', + 'verify-ca', + 'verify-full', + 'no-verify', +] + +let warnedNoVerify = false + +/** Test hook: reset the once-per-process no-verify warning. */ +export function resetNoVerifyWarningForTests(): void { + warnedNoVerify = false +} + +export function buildPgClientConfig( + databaseUrl: string, + extra: Omit = {}, +): pg.ClientConfig { + let url: URL + try { + url = new URL(databaseUrl) + } catch { + // Not URL-parseable (e.g. a bare socket path) — let pg have it verbatim. + return { ...extra, connectionString: databaseUrl } + } + + const params = url.searchParams + if (PASSTHROUGH_PARAMS.some((name) => params.has(name))) { + return { ...extra, connectionString: databaseUrl } + } + let sslmode = params.get('sslmode') + const sslrootcert = params.get('sslrootcert') + if (sslmode === null && sslrootcert === null) { + // Environment tier, mirroring node-postgres's own PGSSLMODE handling + // (connection-parameters.js `readSSLConfigFromEnvironment`) — pg enables + // TLS from this variable but ignores PGSSLROOTCERT entirely, so without + // taking this branch ourselves an env-configured connection would verify + // against the wrong trust anchors. URL parameters win when present + // (libpq precedence); an unset or unrecognised PGSSLMODE stays a pure + // passthrough. + const envMode = process.env.PGSSLMODE + if (envMode !== undefined && ENV_SSLMODES.includes(envMode)) { + sslmode = envMode + } else { + return { ...extra, connectionString: databaseUrl } + } + } + + for (const name of HANDLED_PARAMS) params.delete(name) + const stripped = url.toString() + + if (sslmode === 'disable') { + return { ...extra, connectionString: stripped, ssl: false } + } + + if (sslmode === 'no-verify') { + if (!warnedNoVerify) { + warnedNoVerify = true + process.stderr.write( + 'stash: sslmode=no-verify — the connection to the database is encrypted but the server is NOT authenticated. Prefer sslmode=verify-full with sslrootcert=.\n', + ) + } + return { + ...extra, + connectionString: stripped, + ssl: { rejectUnauthorized: false }, + } + } + + // Everything else — `require`, `verify-ca`, `prefer`, `verify-full`, or a + // bare `sslrootcert` — verifies fully, which is what node-postgres already + // did for these modes (its "aliases for verify-full" advisory). We are + // preserving behaviour, not tightening it. + return { + ...extra, + connectionString: stripped, + ssl: { rejectUnauthorized: true, ca: resolveCa(sslrootcert, url.hostname) }, + } +} + +/** + * CA resolution, first hit wins: + * + * 1. `sslrootcert=` from the URL — libpq semantics: the named file is + * the ONLY trust anchor. `sslrootcert=system` selects the system store. + * 2. `PGSSLROOTCERT` env — same semantics. + * 3. A Supabase host — the bundled Supabase root CA, APPENDED to the system + * roots so publicly-signed certificates keep verifying. + * 4. Otherwise the system trust store (`ca: undefined`). + */ +function resolveCa( + sslrootcert: string | null, + hostname: string, +): string | string[] | undefined { + const explicit = sslrootcert ?? process.env.PGSSLROOTCERT?.trim() + if (explicit) { + if (explicit === 'system') return undefined + try { + return readFileSync(explicit, 'utf-8') + } catch (error) { + const detail = error instanceof Error ? error.message : String(error) + throw new Error( + `Cannot read the CA file named by sslrootcert (${explicit}): ${detail}`, + { cause: error }, + ) + } + } + if (SUPABASE_HOST_PATTERN.test(hostname)) { + return [...tls.rootCertificates, SUPABASE_ROOT_CA_PEM] + } + return undefined +} + +/** Error codes / messages that mean "certificate verification failed". */ +const TLS_FAILURE_PATTERNS = [ + 'SELF_SIGNED_CERT_IN_CHAIN', + 'self-signed certificate', + 'self signed certificate', + 'UNABLE_TO_VERIFY_LEAF_SIGNATURE', + 'unable to verify the first certificate', + 'UNABLE_TO_GET_ISSUER_CERT', + 'unable to get local issuer certificate', + 'CERT_HAS_EXPIRED', + 'certificate has expired', + 'ERR_TLS_CERT_ALTNAME_INVALID', + 'Hostname/IP does not match', +] + +/** + * A certificate-verification failure, re-thrown by the factory's connect + * wrapper with the shaped remedy as its message. Call sites that add their + * own "Failed to connect" framing should rethrow/print this one verbatim — + * the message is self-contained, and re-shaping it nests explanations. + */ +export class TlsVerificationError extends Error {} + +/** + * When `error` is a certificate-verification failure, return a message that + * names the host and the supported remedies in order — so the discoverable + * fix is never `NODE_TLS_REJECT_UNAUTHORIZED=0` (process-wide: it would also + * disable verification for the connections carrying ZeroKMS credentials). + * Returns null for anything that is not a TLS trust failure. + */ +export function explainTlsError( + error: unknown, + databaseUrl: string, +): string | null { + const err = error as { code?: unknown; message?: unknown } | null + const code = typeof err?.code === 'string' ? err.code : '' + const message = typeof err?.message === 'string' ? err.message : '' + const matched = TLS_FAILURE_PATTERNS.some( + (pattern) => code === pattern || message.includes(pattern), + ) + if (!matched) return null + + let host = 'the database host' + try { + host = new URL(databaseUrl).hostname || host + } catch { + // keep the placeholder + } + return [ + `TLS certificate verification failed for ${host}: ${message || code}.`, + 'Fixes, in order of preference:', + " 1. Verify against your provider's CA: append sslrootcert=/path/to/ca.pem to the connection string (or set PGSSLROOTCERT). Supabase hosts are already covered by the CLI's bundled Supabase root CA.", + ' 2. Last resort: sslmode=no-verify keeps the connection encrypted but skips server authentication for THIS connection only.', + 'Never set NODE_TLS_REJECT_UNAUTHORIZED=0 — it disables TLS verification for every connection in the process, including the ones carrying CipherStash credentials.', + ].join('\n') +} diff --git a/packages/cli/src/db/supabase-ca.ts b/packages/cli/src/db/supabase-ca.ts new file mode 100644 index 000000000..35e7162d0 --- /dev/null +++ b/packages/cli/src/db/supabase-ca.ts @@ -0,0 +1,47 @@ +/** + * The Supabase root CA ("Supabase Root 2021 CA"), vendored so + * `sslmode=verify-full` against Supabase databases and poolers works out of + * the box — Supabase signs its Postgres server certificates with a private + * CA, so system trust stores cannot verify them and node-postgres fails with + * `self-signed certificate in certificate chain`. + * + * Provenance (2026-08-18): captured from a live TLS handshake with + * `aws-0-us-east-1.pooler.supabase.com:5432` (the chain includes the + * self-signed root) and verified byte-identical (DER) to the copy Supabase + * vendors in its own CLI + * (github.com/supabase/cli: apps/cli-go/internal/gen/types/templates/prod-ca-2021.crt). + * + * Subject: C=US, ST=Delware, L=New Castle, O=Supabase Inc, + * CN=Supabase Root 2021 CA + * SHA-256: 80:70:25:AD:50:D4:ED:21:9D:2C:9C:7D:29:9C:00:4F: + * 82:4E:B0:0C:F7:F6:5A:FE:F6:07:D0:7B:72:E6:CA:FA + * Expires: 2031-04-26 + * + * Rotation: re-run the capture + cross-check above. The CA is only ever + * APPENDED to the system trust roots (see `resolveCa` in config.ts), so a + * future Supabase move to a publicly-trusted CA keeps verifying. + */ +export const SUPABASE_ROOT_CA_PEM = `-----BEGIN CERTIFICATE----- +MIIDxDCCAqygAwIBAgIUbLxMod62P2ktCiAkxnKJwtE9VPYwDQYJKoZIhvcNAQEL +BQAwazELMAkGA1UEBhMCVVMxEDAOBgNVBAgMB0RlbHdhcmUxEzARBgNVBAcMCk5l +dyBDYXN0bGUxFTATBgNVBAoMDFN1cGFiYXNlIEluYzEeMBwGA1UEAwwVU3VwYWJh +c2UgUm9vdCAyMDIxIENBMB4XDTIxMDQyODEwNTY1M1oXDTMxMDQyNjEwNTY1M1ow +azELMAkGA1UEBhMCVVMxEDAOBgNVBAgMB0RlbHdhcmUxEzARBgNVBAcMCk5ldyBD +YXN0bGUxFTATBgNVBAoMDFN1cGFiYXNlIEluYzEeMBwGA1UEAwwVU3VwYWJhc2Ug +Um9vdCAyMDIxIENBMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqQXW +QyHOB+qR2GJobCq/CBmQ40G0oDmCC3mzVnn8sv4XNeWtE5XcEL0uVih7Jo4Dkx1Q +DmGHBH1zDfgs2qXiLb6xpw/CKQPypZW1JssOTMIfQppNQ87K75Ya0p25Y3ePS2t2 +GtvHxNjUV6kjOZjEn2yWEcBdpOVCUYBVFBNMB4YBHkNRDa/+S4uywAoaTWnCJLUi +cvTlHmMw6xSQQn1UfRQHk50DMCEJ7Cy1RxrZJrkXXRP3LqQL2ijJ6F4yMfh+Gyb4 +O4XajoVj/+R4GwywKYrrS8PrSNtwxr5StlQO8zIQUSMiq26wM8mgELFlS/32Uclt +NaQ1xBRizkzpZct9DwIDAQABo2AwXjALBgNVHQ8EBAMCAQYwHQYDVR0OBBYEFKjX +uXY32CztkhImng4yJNUtaUYsMB8GA1UdIwQYMBaAFKjXuXY32CztkhImng4yJNUt +aUYsMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBAB8spzNn+4VU +tVxbdMaX+39Z50sc7uATmus16jmmHjhIHz+l/9GlJ5KqAMOx26mPZgfzG7oneL2b +VW+WgYUkTT3XEPFWnTp2RJwQao8/tYPXWEJDc0WVQHrpmnWOFKU/d3MqBgBm5y+6 +jB81TU/RG2rVerPDWP+1MMcNNy0491CTL5XQZ7JfDJJ9CCmXSdtTl4uUQnSuv/Qx +Cea13BX2ZgJc7Au30vihLhub52De4P/4gonKsNHYdbWjg7OWKwNv/zitGDVDB9Y2 +CMTyZKG3XEu5Ghl1LEnI3QmEKsqaCLv12BnVjbkSeZsMnevJPs1Ye6TjjJwdik5P +o/bKiIz+Fq8= +-----END CERTIFICATE----- +` diff --git a/packages/cli/src/installer/index.ts b/packages/cli/src/installer/index.ts index 4b41fd33c..c8deec5b2 100644 --- a/packages/cli/src/installer/index.ts +++ b/packages/cli/src/installer/index.ts @@ -1,5 +1,6 @@ import { readInstallSql } from '@cipherstash/eql/sql' -import pg from 'pg' +import type pg from 'pg' +import { createPgClient, TlsVerificationError } from '@/db/client.js' import { DEFERRED_GRANTS_HEADER, EQL_V3_INTERNAL_SCHEMA_NAME, @@ -154,12 +155,15 @@ export class EQLInstaller { } async preflight(): Promise { - const client = new pg.Client({ connectionString: this.databaseUrl }) + const client = createPgClient(this.databaseUrl) try { await client.connect() } catch (error) { - const detail = error instanceof Error ? error.message : String(error) await client.end().catch(() => {}) + // Already shaped centrally by createPgClient's connect wrapper — the + // message is self-contained; adding framing would bury the remedy. + if (error instanceof TlsVerificationError) throw error + const detail = error instanceof Error ? error.message : String(error) throw new Error(`Failed to connect to database: ${detail}`, { cause: error, }) @@ -260,7 +264,7 @@ export class EQLInstaller { /** Generation-aware read-only detection retained for legacy diagnostics. */ async isInstalled(options?: { eqlVersion?: EqlVersion }): Promise { - const client = new pg.Client({ connectionString: this.databaseUrl }) + const client = createPgClient(this.databaseUrl) const requiredSchemas = (options?.eqlVersion ?? 3) === 3 ? [EQL_V3_SCHEMA_NAME, EQL_V3_INTERNAL_SCHEMA_NAME] @@ -273,6 +277,7 @@ export class EQLInstaller { ) return result.rows[0]?.found === requiredSchemas.length } catch (error) { + if (error instanceof TlsVerificationError) throw error const detail = error instanceof Error ? error.message : String(error) throw new Error(`Failed to connect to database: ${detail}`, { cause: error, @@ -288,7 +293,7 @@ export class EQLInstaller { }): Promise { const schemaName = (options?.eqlVersion ?? 3) === 3 ? EQL_V3_SCHEMA_NAME : EQL_V2_SCHEMA_NAME - const client = new pg.Client({ connectionString: this.databaseUrl }) + const client = createPgClient(this.databaseUrl) try { await client.connect() const schemaResult = await client.query( @@ -310,6 +315,7 @@ export class EQLInstaller { } return 'unknown' } catch (error) { + if (error instanceof TlsVerificationError) throw error const detail = error instanceof Error ? error.message : String(error) throw new Error(`Failed to connect to database: ${detail}`, { cause: error, @@ -334,10 +340,11 @@ export class EQLInstaller { * install/upgrade — the install is complete without them. */ async install(options?: { supabase?: boolean }): Promise { - const client = new pg.Client({ connectionString: this.databaseUrl }) + const client = createPgClient(this.databaseUrl) try { await client.connect() } catch (error) { + if (error instanceof TlsVerificationError) throw error const detail = error instanceof Error ? error.message : String(error) throw new Error(`Failed to connect to database: ${detail}`, { cause: error, @@ -381,12 +388,15 @@ export class EQLInstaller { * path, so a plain re-run recovers without `--force`. */ async applySupabaseGrants(): Promise { - const client = new pg.Client({ connectionString: this.databaseUrl }) + const client = createPgClient(this.databaseUrl) try { await client.connect() } catch (error) { - const detail = error instanceof Error ? error.message : String(error) await client.end().catch(() => {}) + // Already shaped centrally by createPgClient's connect wrapper — the + // message is self-contained; adding framing would bury the remedy. + if (error instanceof TlsVerificationError) throw error + const detail = error instanceof Error ? error.message : String(error) throw new Error(`Failed to connect to database: ${detail}`, { cause: error, }) diff --git a/skills/stash-cli/SKILL.md b/skills/stash-cli/SKILL.md index 5953fec6e..8963eee08 100644 --- a/skills/stash-cli/SKILL.md +++ b/skills/stash-cli/SKILL.md @@ -157,6 +157,16 @@ First hit wins: The resolved URL is returned in memory only. It is never written to disk or into `process.env`. +### TLS to the database + +Every CLI database connection honours `sslmode` and `sslrootcert` from the connection string, and the `PGSSLMODE` / `PGSSLROOTCERT` environment variables when the URL carries no TLS parameters (URL parameters win, libpq precedence — and unlike raw node-postgres, `PGSSLROOTCERT` is actually consumed): + +- `sslmode=verify-full` (and `require` / `verify-ca` / `prefer`, which the CLI treats identically — full verification, matching node-postgres's current behaviour) verifies the server certificate. CA resolution order: `sslrootcert=` in the URL (libpq semantics — that file becomes the only trust anchor; `sslrootcert=system` selects the system store) → the `PGSSLROOTCERT` environment variable → for `*.supabase.co` / `*.supabase.com` hosts, the CLI's **bundled Supabase root CA** (appended to the system roots) → the system trust store. +- Supabase therefore verifies out of the box — direct hosts and the pgBouncer pooler alike. No certificate download needed. +- `sslmode=no-verify` is honoured but prints a one-line stderr warning: the connection is encrypted, the server is not authenticated. `sslmode=disable` turns TLS off. +- **Never set `NODE_TLS_REJECT_UNAUTHORIZED=0`** — it disables TLS verification for every connection in the process, including the ones carrying CipherStash credentials. A certificate-verification failure from any CLI command names the host and the supported remedies (shaped centrally in the connection factory); follow those instead. +- URLs using client certificates (`sslcert` / `sslkey`) or the raw `ssl` param are passed to node-postgres untouched. + ## Telemetry The CLI collects **anonymous, opt-out** usage analytics — coarse events only diff --git a/skills/stash-supabase/SKILL.md b/skills/stash-supabase/SKILL.md index c605a4b35..f6609cd13 100644 --- a/skills/stash-supabase/SKILL.md +++ b/skills/stash-supabase/SKILL.md @@ -101,6 +101,12 @@ supabase db push # remote/linked project > preflight` (`--json` for agents), which reports membership of `postgres` > alongside the other role capabilities. +> **TLS:** the CLI bundles the Supabase root CA, so `sslmode=verify-full` +> against Supabase hosts (direct and pooler) verifies out of the box — no +> certificate download, and never `NODE_TLS_REJECT_UNAUTHORIZED=0` (it is +> process-wide and would also disable verification for CipherStash credential +> traffic). A supplied `sslrootcert=` or `PGSSLROOTCERT` still wins. + The generated file carries three things, in order: the EQL v3 bundle, the role grants, and the `cipherstash.cs_migrations` tracking schema that `stash encrypt` records per-column progress in. One `supabase db reset` therefore