fix(cli): treat only verifying sslmodes as a local TLS demand (CLI-2366)

[Coly010]

Summary

Follow-up to #6652, addressing @avallete's post-merge review
(review).

#6652 stopped sslConfigsFor from silently downgrading an explicit sslmode=require /
verify-full to plaintext on a target classified local. The predicate it introduced was too broad:
it treated any populated sslmode as a TLS demand.

That is not the same as the DSN asking for TLS:

• prefer and allow name a TLS/plaintext fallback that libpq performs and sslConfigsFor
  deliberately does not — it maps prefer and unset to TLS-only. Local classification used to force
  plaintext before that mapping ran, so reading prefer as a demand turns a plaintext-capable
  target into a handshake failure.
• sslmode is also filled from PGSSLMODE and libpq service files when the DSN omits it, so a
  merely present value can come from the environment rather than the connection string.

The result was a regression on the shared DbConnection seam: a developer with PGSSLMODE=prefer
exported, or a DSN carrying ?sslmode=prefer, could no longer reach a local plaintext Postgres —
affecting db query --db-url, dump and pull as well as gen types.

Only require, verify-ca, verify-full, or a supplied sslrootcert / inline CA now count as a
demand.

Verified against a plaintext Postgres on the local-classified port

Connection	merged develop	this branch
bare loopback DSN	connects	connects
PGSSLMODE=prefer	fails	connects
PGSSLMODE=allow	fails	connects
?sslmode=prefer	fails	connects
?sslmode=disable	connects	connects
?sslmode=require	fails closed	fails closed
PGSSLMODE=verify-full	fails closed	fails closed

The TLS-tunnel behaviour #6652 set out to fix is unchanged: an explicit demand still puts an
SSLRequest on the wire against a loopback target, and a supplied root cert still opens the CA gate.

Coverage is behavioural rather than shape-only — the integration tests assert whether an SSLRequest
actually reaches a fake Postgres server, and a unit test composes the real DSN parser with the
predicate so the PGSSLMODE-filled case is covered end to end rather than assumed.

Linked issue

Follow-up to CLI-2366 (no GitHub issue).

• The linked issue is open and carries the open-for-contribution label (or I'm a Supabase maintainer).

Checklist

• The PR title follows Conventional Commits (e.g. fix(cli): …).
• Tests added or updated for the change.
• From the repository root, pnpm check:all passes; relevant package tests pass for every touched workspace, and pnpm types:check passes for each touched TypeScript workspace (or workspace declaring it).

🤖 Generated with Claude Code

[github-actions]

🤖 AI Review

Both independent reviews completed. The core fix addresses ambient PGSSLMODE=prefer, but its provenance-blind mode check causes a major regression for explicitly configured local prefer/allow connections and leaves the analogous ambient PGSSLROOTCERT problem. Six findings are confirmed; the import-order nit is refuted because no trusted convention requires that ordering.

Findings

Severity	Location	Category	Sources	Claim
🟠 MAJOR	apps/cli/src/command-internal/db-connection.sql-pg.layer.ts:415	tls-semantics	claude+codex	Treating local sslmode=prefer and sslmode=allow as non-TLS-demanding makes explicitly configured local targets plaintext-only, breaking TLS-only loopback endpoints and suppressing allow's implemented plaintext-then-TLS fallback.
🟡 MINOR	apps/cli/src/command-internal/db-connection.sql-pg.layer.ts:416	correctness	claude	An ambient PGSSLROOTCERT still makes a bare local connection demand TLS, despite sslrootcert having the same environment/service provenance ambiguity used to justify discounting ambient sslmode.
⚪ NIT	apps/cli/src/command-internal/db-connection.sql-pg.integration.test.ts:913	test-quality	claude	The new allow integration test cannot validate the PR's changed allow classification because the same plaintext-server assertion passes before and after the change, and its title incorrectly says sslConfigsFor does not implement allow's fallback.
⚪ NIT	apps/cli/src/command-internal/db-connection.sql-pg.unit.test.ts:283	comments	claude+codex	The new test title/comments falsely state that sslConfigsFor never performs allow's fallback, and the comments add narration and ticket-provenance language contrary to trusted repository guidance.
⚪ NIT	apps/cli/src/command-internal/db-connection.sql-pg.unit.test.ts:308	test-quality	claude	The verify-full environment test dereferences the optional parser result without first asserting successful parsing, so parser failures produce an unhelpful runtime error.
⚪ NIT	apps/cli/src/command-internal/db-connection.sql-pg.layer.ts:468	documentation	claude	The sslConfigsFor documentation still says every explicitly set sslmode removes the local TLS exemption, which is no longer true for prefer, allow, or disable.

Findings outside the diff

• ⚪ NIT apps/cli/src/command-internal/db-connection.sql-pg.layer.ts:468 — The sslConfigsFor documentation still says every explicitly set sslmode removes the local TLS exemption, which is no longer true for prefer, allow, or disable.

Refuted findings (kept for transparency, not posted as review comments)

• apps/cli/src/command-internal/db-connection.sql-pg.unit.test.ts:28 (style): The new relative import violates the repository's alphabetical import ordering convention.
  Refuted: Neither trusted/CLAUDE.md nor trusted/apps/cli/CLAUDE.md requires alphabetical import paths, and the trusted baseline of this same file already places db-connection.sql-pg.layer.ts before db-connection.service.ts, which is itself non-alphabetical. The cited ordering therefore is not an established repository convention.

Stats

Claude findings: 7 · Codex findings: 2 · Confirmed: 6 · Refuted: 1 · Uncertain: 0

---

Models: claude-opus-5 + gpt-5.6-sol · Trigger: auto · Workflow run

This review runs once per PR. A maintainer can request another with a /ai-review comment.