Skip to content

fix(postgres): respect sslmode from connection string#191

Merged
Yostra merged 2 commits into
v2from
fix-postgres-tls-semantics
Mar 30, 2026
Merged

fix(postgres): respect sslmode from connection string#191
Yostra merged 2 commits into
v2from
fix-postgres-tls-semantics

Conversation

@tonyxiao
Copy link
Copy Markdown
Collaborator

Summary

841d023 (from #185) hardcoded ssl: { rejectUnauthorized: false } for all Postgres connection strings. This silently overrides any sslmode the caller set — e.g. a user with sslmode=verify-full on an RDS instance would have cert verification quietly disabled.

This PR resolves the TODO: Preserve connection-string sslmode semantics in all 4 places by parsing the sslmode query parameter:

sslmode ssl option
disable false
verify-ca / verify-full true (verify against system CA)
require / prefer / unset { rejectUnauthorized: false } (TLS on, no cert check — safe for proxied connections)

Applied in:

  • destination-postgres buildPoolConfig
  • state-postgres setupStateStore, createStateStore, runMigrationsWithContent

Test plan

  • cd packages/destination-postgres && pnpm test — new sslmode=disable, sslmode=verify-full, sslmode=require cases pass
  • pnpm build — clean

cc @kdhillon-stripe — this resolves the TODO left in 841d023. The rejectUnauthorized: false default is preserved for unset sslmode (correct for proxied connections), but verify-full / disable are now respected.

🤖 Generated with Claude Code

tonyxiao and others added 2 commits March 29, 2026 18:02
…ing rejectUnauthorized:false

The previous commit (841d023) hardcoded ssl: { rejectUnauthorized: false }
for all connection strings, silently overriding any sslmode the caller
set (e.g. sslmode=verify-full on an RDS instance).

Parse the sslmode query parameter and map it to the correct pg ssl option:
- disable          → ssl: false
- verify-ca/full   → ssl: true  (verify cert against system CA)
- require/prefer/unset → ssl: { rejectUnauthorized: false }  (TLS on, no cert check — safe for proxied connections)

Applied in destination-postgres buildPoolConfig, state-postgres
setupStateStore, createStateStore, and runMigrationsWithContent.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Committed-By-Agent: claude
…rate-openapi.sh --config

SSL is now opt-in: connections without an explicit sslmode use ssl:false
by default. Use sslmode=require for SSL without cert verification, or
sslmode=verify-ca / verify-full for full verification.

Also fixes generate-openapi.sh to pass --config .prettierrc when writing
to a temp dir (--check mode), so prettier uses printWidth:100 from the
project config instead of the default 80. This resolves persistent DRIFT
detection on CI where the enum array was wrapped differently.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Committed-By-Agent: claude
@tonyxiao tonyxiao force-pushed the fix-postgres-tls-semantics branch from b8542de to d1bf372 Compare March 30, 2026 01:03
matingathani added a commit to matingathani/stripe-sync-engine that referenced this pull request Mar 30, 2026
The IF NOT EXISTS guards in several migration files checked pg_type by
name only, without constraining by schema. If a type with the same name
happened to exist in another schema (e.g. public), the check would
pass and the migration would skip creating the stripe-schema enum,
causing subsequent column definitions that reference it to fail.

Joined pg_namespace in all affected checks so the guard is limited to
the stripe schema:

- 0003_prices.sql: pricing_type, pricing_tiers
- 0004_subscriptions.sql: subscription_status
- 0005_invoices.sql: invoice_status
- 0024_subscription_schedules.sql: subscription_schedule_status

Fixes stripe#191
Comment on lines +7 to +12
function sslConfigFromConnectionString(connStr: string): ClientConfig['ssl'] {
try {
const sslmode = new URL(connStr).searchParams.get('sslmode')
if (sslmode === 'disable') return false
if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true
if (sslmode === 'require') return { rejectUnauthorized: false }
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tonyxiao can we move this into packages/util-postgres to not duplicate it across files

@Yostra Yostra merged commit 60bf93a into v2 Mar 30, 2026
6 checks passed
@Yostra Yostra deleted the fix-postgres-tls-semantics branch March 30, 2026 16:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants