fix: consume libpq SSL connection string parameters instead of forwarding them - #1551
Conversation
PostgreSQL connection strings carrying the standard libpq SSL parameters (sslrootcert, sslcert, sslkey, sslpassword) failed with "unrecognized configuration parameter" errors because postgres.js forwards unknown URL query parameters to the server as runtime configuration parameters. Add createPostgresJSConnectionConfig to @prisma/studio-core/data/postgresjs, which consumes these parameters client-side: certificate files are read from disk and mapped to TLS options (honoring sslmode verification semantics, including the libpq rule that a root certificate upgrades sslmode=require to verify-ca and that sslrootcert=system forces full verification), and the parameters are stripped from the connection string handed to postgres(). The ppg-dev demo runtime now routes external database URLs through the helper. Fixes #1433 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
|
Compute preview deployed. Branch: |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Warning
CodeRabbit couldn't request changes on this pull request because it doesn't have sufficient GitHub permissions.
Please grant CodeRabbit Pull requests: Read and write permission and re-run the review.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@data/postgresjs/connection-options.test.ts`:
- Around line 202-218: Update createPostgresJSConnectionConfig to reject
sslrootcert=system when sslmode is require, verify-ca, disable, allow, or
prefer, matching libpq’s invalid-combination behavior instead of rewriting or
disabling TLS. Preserve the existing full-verification behavior for the
supported default/verify-full case, and add test coverage for each rejected
sslmode combination.
- Around line 262-272: Update the fixture path construction in the “reads ssl
files from disk by default” test to convert the file URL with fileURLToPath
instead of using URL.pathname, preserving correct decoding and cross-platform
behavior. Add or reuse the appropriate fileURLToPath import and keep the
existing SSL assertion unchanged.
In `@data/postgresjs/connection-options.ts`:
- Around line 91-103: Update the sslMode handling in the connection-string
parsing flow to validate the extracted value before deleting it or constructing
the stripped connection string. Reject any non-null sslmode value that is not
supported by the implementation, rather than silently removing it; preserve the
existing sslMode === "disable" behavior and normal handling for supported
values.
- Around line 141-151: The SSL configuration logic must preserve libpq’s
plaintext-first/fallback negotiation for sslMode “allow” and “prefer” even when
SSL files are configured. Update the surrounding connection-options handling so
these modes are not forced into an always-SSL postgres.js configuration, or
explicitly reject the unsupported combinations; keep existing verification
behavior for “verify-full” and “verify-ca” unchanged.
In `@FEATURES.md`:
- Around line 10-11: Update the PostgreSQL SSL documentation to clarify that
standalone sslmode is not stripped and remains available for postgres.js. State
that only SSL file parameters are removed, along with an accompanying sslmode
when those parameters are translated into client-side TLS configuration.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 73f7ab00-02cd-4249-967f-0a7cb5d3b6e9
📒 Files selected for processing (6)
.changeset/fix-sslrootcert-forwarding.mdFEATURES.mddata/postgresjs/connection-options.test.tsdata/postgresjs/connection-options.tsdata/postgresjs/index.tsdemo/ppg-dev/runtime.ts
- Reject sslrootcert=system combined with any sslmode weaker than verify-full, matching libpq's "weak sslmode disallowed with system CA" behavior, instead of silently rewriting verification or letting sslmode=disable turn TLS off. - Validate sslmode against the known libpq set (disable, allow, prefer, require, verify-ca, verify-full) and throw a descriptive error on unrecognized values instead of silently dropping them. - Reject sslmode=allow/prefer combined with SSL file parameters: postgres.js can only honor plaintext-fallback negotiation for its built-in string modes, not with custom TLS options, so failing fast beats silently forcing TLS on. - Use fileURLToPath(import.meta.url) for the on-disk test fixture path. - Clarify in FEATURES.md that only SSL file parameters (plus an accompanying sslmode) are stripped; a standalone sslmode stays in the URL for postgres.js. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 participant
Fixes #1433
Root cause
postgres.js copies every URL query parameter it doesn't recognize into the startup-packet runtime parameters sent to the server.
sslrootcert,sslcert,sslkey, andsslpasswordaren't in its known-options list, so PostgreSQL rejects the connection withunrecognized configuration parameter "sslrootcert".Fix
New
createPostgresJSConnectionConfig(connectionString, { readFile? })exported from@prisma/studio-core/data/postgresjs. It returns a stripped connection string plus postgres.js options:sslrootcert→ TLSca(withsystem→ Node system trust store, full verification, per libpq)sslcert/sslkey→cert/keyfile contents;sslpassword→passphrasesslmodemapped to libpq-compatible TLS semantics (disable,verify-full,verify-ca,require/prefer/allow, incl. the upgrade-to-verify-ca rule when a root cert is provided)Verification
15 new tests, the first reproducing the raw forwarding against real postgres.js (failing-first). Typecheck/lint clean; changeset included; documented in FEATURES.md.
Follow-up: the Prisma CLI (prisma/prisma) should call this helper before constructing its postgres.js client so
prisma studiousers get the fix.🤖 Generated with Claude Code