Repository navigation
Conversation
There was a problem hiding this comment.
🤖 AI Review
Confirmed the missing malformed-pooler-URL regression test. The TLS downgrade is verified, but the claim of plaintext password transmission remains uncertain because it depends on server authentication and SSL enforcement. Codex reported no findings. Verification was static; tests were not run.
Findings
Stats
Claude findings: 2 · Codex findings: 0 · Confirmed: 1 · Refuted: 0 · Uncertain: 1
Models: claude-opus-5-5 + gpt-6.1-sol · Trigger: auto · Workflow run
This review runs once per PR. A maintainer can request another with a /ai-review comment.
avallete
left a comment
There was a problem hiding this comment.
Dogfooded on a staging project against develop. On the direct host, verify-full with a CA it shouldn't trust, a missing PGSSLROOTCERT file and PGSSLMODE=bogus now all fail as described; develop ignored the first two. require pushes fine.
One non-blocking note on the description: "--linked db commands now read PGSSL*" is broader than the change. db dump, db pull and migration squash build their pg_dump env in pg-dump.env.ts, which forwards only host, port, user, password and database. So db dump --linked with PGSSLMODE=verify-full and an untrusted CA still succeeds. develop has the same gap, so this PR doesn't cause it. Could you narrow the wording to the commands that connect through the resolver? Forwarding SSL settings to pg_dump can be a follow-up.
## TL;DR
remote `db dump` and the first `db pull` seed now pass the resolved TLS
settings to `pg_dump`
## whats introduced?
`pg_dump` only got host, port, user, password and database, so a
container dump ignored `PGSSLMODE=verify-full`, trusted any CA, and got
turned away by servers that require a client cert.
for the linked project and a remote `--db-url`, it now gets the resolved
`sslmode` and connect timeout, the root cert under `require` or
`verify-*`, and the client cert unless the key needs `sslpassword`
In a container the cert files are mounted read only, and a missing one
fails before docker runs
> [!NOTE]
> a remote dump that only worked because verification was skipped now
fails. local targets and runs without TLS or timeout settings are
unchanged
<details>
<summary>visual rep:</summary>
```mermaid
flowchart TD
conn["resolved connection"] --> remote{"remote target?"}
remote -- no --> plain["host, port, user, password, database"]
remote -- yes --> tls["+ sslmode and connect timeout<br/>+ root cert under require or verify-*<br/>+ system root cert forces verify-full<br/>+ client cert unless the key needs sslpassword"]
tls --> where{"where pg_dump runs"}
where -- container --> box["cert files checked, then mounted read only<br/>key copied to a private file<br/>verify-* without root cert, or system, uses the image CA bundle"]
where -- native --> host["host paths kept<br/>verify-* without root cert uses ~/.postgresql/root.crt, else the host CA bundle<br/>system uses the host CA bundle"]
plain --> dump["pg_dump"]
box --> dump
host --> dump
```
</details>
## ref
- spotted while working on: #7261
- related: CLI-2753

TL;DR
supabase db push --linkedstops silently droppingPGSSLMODEandPGSSLROOTCERTwhen the direct host answerswhats introduced?
--linkedcommands that connect from the CLI itself (db push,migrationcommands,inspect db, …now read
PGSSLMODE,PGSSLROOTCERTandPGSSLCERT/PGSSLKEYfrom the shell on the direct host, the way the pooler fallback already did.ps: the
pg_dumpsteps indb dump,db pullandmigration squashstill don't forward them.PGSERVICEandPGCONNECT_TIMEOUTapply too, and a missingPGSSLROOTCERTfile now fails instead of being ignored.host, role and database stay pinned. an invalid mode fails naming the vars on either route, and
sslmode=disablenow dials plaintext without reading cert files.visual rep
supabase db push --linkedPGSSL*ignoredPGSSLMODEwhy this?
--db-urland the pooler fallback already read these vars.the linked direct host was skipped by mistake during the port, so it now reads them too
ref: