fix(quaint): Set TLS 1.2 minimum on macOS to prevent P1011 (issue #29600) - #5848
fix(quaint): Set TLS 1.2 minimum on macOS to prevent P1011 (issue #29600)#5848arab971 wants to merge 5 commits into
Conversation
…ol version macOS Security framework defaults to TLS 1.0 which causes "bad protocol version" errors when connecting to PostgreSQL servers requiring TLS 1.2+ (e.g., Ubicloud Managed Postgres #29600). Add conditional platform guard so only macOS builds enforce TLS 1.2 minimum; Linux/Windows unchanged.
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughPostgres native TLS URLs now parse 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@quaint/src/connector/postgres/native/mod.rs`:
- Around line 925-930: Update MakeTlsConnectorManager::get_connector() to apply
SslParams.tls_min_version and tls_max_version via the native_tls builder,
validating and returning an error when the configured minimum exceeds the
maximum before building. On macOS, use the greater of the TLS 1.2 platform floor
and configured minimum; preserve configured bounds on other platforms.
🪄 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 Plus
Run ID: 28590fb4-634f-4cc0-b464-114bb9d728c1
📒 Files selected for processing (5)
.github/workflows/build-engines-apple-intel-template.yml.github/workflows/build-engines-apple-silicon-template.ymlquaint/Cargo.tomlquaint/src/connector/postgres/native/mod.rsquaint/src/connector/postgres/url.rs
…validation for macOS
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@quaint/src/connector/postgres/native/mod.rs`:
- Around line 893-909: Update string_to_protocol to recognize TLSv1.3 and
TLS1.3, returning native_tls::Protocol::Tlsv13 on non-Apple platforms while
preserving rejection on macOS. Replace any NativeProtocol discriminant casts
used for min/max comparisons with an explicit TLS-version ordering value, and
compare those values instead of relying on enum declaration order.
- Around line 1018-1033: Update the non-macOS TLS setup around tls_builder and
its build call to use the documented native-tls default as the effective minimum
when tls_min is None. Compare that effective minimum against an explicit
tls_max, return the existing InvalidConnectionArguments error when it exceeds
the maximum, and apply the effective minimum to min_protocol_version before
building the connector; retain the explicit Some(min)/Some(max) validation if
still necessary.
🪄 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 Plus
Run ID: 20f2c814-8db9-41ea-9624-38008124d42f
📒 Files selected for processing (2)
quaint/src/connector/postgres/native/mod.rsquaint/src/connector/postgres/url.rs
| /// Converts a TLS version string to a `native_tls::Protocol` enum. | ||
| /// | ||
| /// Supported formats: "TLSv1.0", "TLSv1.1", "TLSv1.2", "TLS1.0", "TLS1.1", "TLS1.2" | ||
| /// (case-insensitive). Returns an error for unrecognized or malformed values. | ||
| fn string_to_protocol(s: &str) -> Result<native_tls::Protocol, String> { | ||
| let normalized = s.to_uppercase(); | ||
| // Only accept documented TLS version forms: | ||
| // - With 'V': "TLSV1.0", "TLSV1.1", "TLSV1.2" | ||
| // - Without 'V': "TLS1.0", "TLS1.1", "TLS1.2" | ||
| match normalized.as_str() { | ||
| "TLSV1.0" | "TLS1.0" => Ok(native_tls::Protocol::Tlsv10), | ||
| "TLSV1.1" | "TLS1.1" => Ok(native_tls::Protocol::Tlsv11), | ||
| "TLSV1.2" | "TLS1.2" => Ok(native_tls::Protocol::Tlsv12), | ||
| _ => Err(format!("Unsupported TLS version: '{}'. Supported versions are: TLSv1.0, TLSv1.1, TLSv1.2", s)), | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
What is the current native-tls crate Protocol enum variant list and does it implement PartialOrd, as of the latest release?
💡 Result:
As of the latest release, the Protocol enum in the native-tls crate consists of the following variants [1][2]: - Sslv3 - Tlsv10 - Tlsv11 - Tlsv12 - Tlsv13 The enum is marked as #[non_exhaustive] [1][2], meaning additional variants may be added in future versions. Regarding its trait implementations, the Protocol enum does not implement the PartialOrd trait [2][3]. According to the source code, it derives only Debug, Copy, and Clone [2][3].
Citations:
- 1: https://docs.rs/native-tls/latest/native_tls/enum.Protocol.html
- 2: https://github.com/rust-native-tls/rust-native-tls/blob/main/src/lib.rs
- 3: https://github.com/rust-native-tls/rust-native-tls/blob/master/src/lib.rs
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Cargo files =="
git ls-files | rg '(^|/)(Cargo\.toml|Cargo\.lock)$' || true
echo
echo "== native-tls references =="
rg -n 'native-tls|native_tls|Protocol::|as u32|tls_min|tls_max_version|string_to_protocol' -S .
echo
echo "== relevant Cargo manifest snippets =="
for f in $(git ls-files | rg '(^|/)Cargo\.toml$'); do
if rg -q 'native-tls|native_tls' "$f"; then
echo "--- $f ---"
rg -n -C 3 'native-tls|native_tls' "$f"
fi
done
echo
echo "== lockfile version entry =="
if [ -f Cargo.lock ]; then
rg -n -C 4 'name = "native-tls"|version = ' Cargo.lock | sed -n '1,120p'
fi
echo
echo "== target file slice =="
if [ -f quaint/src/connector/postgres/native/mod.rs ]; then
wc -l quaint/src/connector/postgres/native/mod.rs
sed -n '860,1050p' quaint/src/connector/postgres/native/mod.rs
fiRepository: prisma/prisma-engines
Length of output: 45468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Cargo.lock native-tls entries =="
python3 - <<'PY'
from pathlib import Path
p=Path('Cargo.lock')
text=p.read_text()
lines=text.splitlines()
for i,l in enumerate(lines):
if l=="[[package]]" and i+2 < len(lines) and 'name = "native-tls"' in lines[i+1]:
start=max(0,i-4)
end=min(len(lines),i+50)
print('--- native-tls package ---')
for n in range(start,end):
print(f'{n+1:4}: {lines[n]}')
PY
echo
echo "== all native-tls versions in lockfile (if duplicated) =="
python3 - <<'PY'
from pathlib import Path
p=Path('Cargo.lock')
lines=p.read_text().splitlines()
for i,l in enumerate(lines):
if l=="[[package]]":
name=None
version=None
source=None
for j in range(i+1, min(len(lines), i+30)):
if lines[j].startswith('name = '): name=lines[j].split('"',2)[1]
if name=="native-tls":
if lines[j].startswith('version = '): version=lines[j].split('"',2)[1]
if lines[j].startswith('source = '): source=lines[j].split('"',2)[1]
print(version, source)
break
PY
echo
echo "== workspace features/native-tls references in Cargo.tomls =="
rg -n 'native-tls|native_tls' --glob 'Cargo.toml' --glob 'Cargo.lock' -SRepository: prisma/prisma-engines
Length of output: 1956
🌐 Web query:
native-tls 0.2.14 Protocol enum variants Tlsv13 as u32 PartialOrd
💡 Result:
In the native-tls crate (including version 0.2.14), the Protocol enum represents SSL/TLS protocol versions [1][2]. The enum is defined as follows: #[non_exhaustive] pub enum Protocol { Sslv3, Tlsv10, Tlsv11, Tlsv12, Tlsv13, } Regarding your specific questions: 1. Protocol Enum Variants: The Protocol enum includes Sslv3, Tlsv10, Tlsv11, Tlsv12, and Tlsv13 [1][3]. 2. Tlsv13: This variant represents the TLS 1.3 protocol [1]. It is important to note that Tlsv13 is not supported on macOS/iOS (Apple platforms); on these platforms, the implementation will either fall back to TLS 1.2 or fail with an error [1][3]. 3. PartialOrd and u32: The native-tls Protocol enum does not automatically derive PartialOrd, nor is it natively represented as a u32 type in the public API [3][4]. While you can manually implement or map these values to an integer type if needed for custom ordering or logic, the library itself provides the enum as a distinct type without these trait implementations by default [3][4]. If you are encountering issues with Tlsv13 in version 0.2.14, be aware that there have been reports of implementation issues with this version, leading some projects to lock to version 0.2.15 or alternative configurations [5].
Citations:
- 1: https://docs.rs/native-tls/latest/native_tls/enum.Protocol.html
- 2: https://docs.rs/crate/native-tls/0.2.14
- 3: https://github.com/rust-native-tls/rust-native-tls/blob/master/src/lib.rs
- 4: https://github.com/rust-native-tls/rust-native-tls/blob/main/src/lib.rs
- 5: Support TLS 1.3? rust-native-tls/rust-native-tls#140
🌐 Web query:
github rust-native-tls Protocol rust 0.2.14 derives Clone Copy Debug Ord PartialOrd
💡 Result:
In the native-tls crate version 0.2.14, the Protocol enum is defined with the following derives: Debug, Copy, and Clone [1][2]. It does not derive Ord or PartialOrd [1][2]. The definition is as follows: #[derive(Debug, Copy, Clone)] #[non_exhaustive] pub enum Protocol { Sslv3, Tlsv10, Tlsv11, Tlsv12, Tlsv13, } As of version 0.2.14, Ord and PartialOrd are not implemented for the Protocol enum [1][2].
Citations:
- 1: https://github.com/rust-native-tls/rust-native-tls/blob/master/src/lib.rs
- 2: https://github.com/rust-native-tls/rust-native-tls/blob/main/src/lib.rs
Add TLS 1.3 support and avoid ordering Protocol by discriminant.
native-tls 0.2.14 already includes Protocol::Tlsv13, so "TLSv1.3"/"TLS1.3" should be accepted on non-Apple platforms while remaining rejected on macOS where the native backend does not support it. Also, NativeProtocol is not Ord/PartialOrd; the min as u32 > max as u32 checks depend on declaration-order discriminants, so map the protocols to an explicit TLS-version enum/value before comparing bounds.
🤖 Prompt for 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.
In `@quaint/src/connector/postgres/native/mod.rs` around lines 893 - 909, Update
string_to_protocol to recognize TLSv1.3 and TLS1.3, returning
native_tls::Protocol::Tlsv13 on non-Apple platforms while preserving rejection
on macOS. Replace any NativeProtocol discriminant casts used for min/max
comparisons with an explicit TLS-version ordering value, and compare those
values instead of relying on enum declaration order.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@quaint/src/connector/postgres/native/mod.rs`:
- Around line 1090-1094: Add connector-level TLS configuration tests alongside
rejects_tls13_with_neutral_error_message covering invalid minimum/maximum bounds
and tls_min_version greater than tls_max_version. Add platform-specific tests
for the non-macOS default minimum and the macOS TLS 1.2 floor, ensuring each
platform case runs only on its applicable target.
🪄 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 Plus
Run ID: 8ccaf80a-558c-4c9d-a153-451d8c95e7e0
📒 Files selected for processing (1)
quaint/src/connector/postgres/native/mod.rs
…nd add validation tests for TLS version bounds
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@quaint/src/connector/postgres/native/mod.rs`:
- Around line 1175-1185: Update tls_valid_bounds_pass_validation to call the
successful connector path directly instead of get_connector_err, asserting that
get_connector succeeds for valid TLS bounds. Preserve validation-error checks
for invalid-bound tests and avoid treating successful construction as a failure.
🪄 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 Plus
Run ID: 9a6ad90d-81dc-42fb-940a-526fba8397e3
📒 Files selected for processing (1)
quaint/src/connector/postgres/native/mod.rs
| async fn tls_valid_bounds_pass_validation() { | ||
| let url = make_tls_with_bounds(Some("TLSv1.0"), Some("TLSv1.2")); | ||
| let err = get_connector_err(url).await; | ||
| // Valid bounds pass the validation layer; the call fails later at | ||
| // tls_builder.build() because there is no real TLS backend in unit tests. | ||
| assert!( | ||
| !err.contains("tls_min_version") | ||
| && !err.contains("tls_max_version") | ||
| && !err.contains("Invalid TLS configuration"), | ||
| "unexpected validation error: {err}" | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Locate file and relevant symbols"
wc -l quaint/src/connector/postgres/native/mod.rs || true
rg -n "tls_valid_bounds_pass_validation|get_connector_err|get_connector|MakeTlsConnectorManager|make_tls_with_bounds" quaint/src/connector/postgres/native/mod.rs
echo "## Relevant sections"
sed - n '1130,1215p' quaint/src/connector/postgres/native/mod.rs
sed - n '1020,1095p' quaint/src/connector/postgres/native/mod.rsRepository: prisma/prisma-engines
Length of output: 2650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
p = Path('quaint/src/connector/postgres/native/mod.rs')
s = p.read_text()
checks = {
"test_name": "async fn tls_valid_bounds_pass_validation() {" in s,
"caller_has_asserts_only": "let err = get_connector_err(url).await;" in s,
}
for name, ok in checks.items():
print(f"{name}: {ok}")
PYRepository: prisma/prisma-engines
Length of output: 205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## Relevant sections with sed"
sed -n '1108,1184p' quaint/src/connector/postgres/native/mod.rs
echo "## get_connector and TLS builder implementation"
sed -n '928,970p' quaint/src/connector/postgres/native/mod.rs
echo "## build-related references"
rg -n "build\(\)|tls_min_version|tls_max_version|Invalid TLS|MakeTlsConnector" quaint/src/connector/postgres/native/mod.rsRepository: prisma/prisma-engines
Length of output: 8376
Test the successful connector path.
get_connector_err() panics when get_connector() succeeds, so valid TLS bounds make this test fail whenever MakeTlsConnector::new(tls_builder.build()?) constructs successfully. This test should assert the connector is built successfully instead.
🤖 Prompt for 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.
In `@quaint/src/connector/postgres/native/mod.rs` around lines 1175 - 1185, Update
tls_valid_bounds_pass_validation to call the successful connector path directly
instead of get_connector_err, asserting that get_connector succeeds for valid
TLS bounds. Preserve validation-error checks for invalid-bound tests and avoid
treating successful construction as a failure.
2 participants
📌 Repository
│ This PR targets prisma/prisma-engines (https://github.com/prisma/prisma-engines), NOT prisma/prisma
│ (https://github.com/prisma/prisma).
│
│ The Schema Engine is implemented in Rust and lives in the prisma-engines repo. The main prisma/prisma repo is a
│ Node.js wrapper that calls the Schema Engine binary. The fix must be applied to the Rust code in quaint (the
│ Postgres connector used by the Schema Engine), then a new Schema Engine binary will be released and consumed by
│ prisma/prisma.
────────────────────────────────────────────────────────────────────────────────
🐛 Bug Description
prisma migrate deploy fails on macOS with:
…when connecting to PostgreSQL servers requiring TLS 1.2+ (e.g., Ubicloud Managed Postgres, AWS RDS with
rds-ca-rsa2048-g1, modern cloud Postgres providers).
The same database, same credentials, same schema work fine on Linux and through @prisma/adapter-pg, indicating a
platform-specific TLS connection failure isolated to the Schema Engine's native TLS path on macOS.
Original user issue (filed against prisma/prisma):
👉 prisma/prisma#29600
────────────────────────────────────────────────────────────────────────────────
🔍 Root Cause Analysis
macOS uses Apple's Security framework via the native-tls Rust crate for TLS. Unlike Linux/Windows where OpenSSL
defaults to minimum TLS version 1.2, the macOS Security framework defaults to TLS 1.0.
Bounds investigation
┌────────────────────────────────────────┬──────────────────────┬───────────────────┬─────────────────────────────┐
│ Component │ macOS TLS default │ Linux TLS default │ Result │
├────────────────────────────────────────┼──────────────────────┼───────────────────┼─────────────────────────────┤
│ native-tls (Security framework) │ TLS 1.0 │ OpenSSL 1.2+ │ Bad protocol version on │
│ │ │ │ macOS │
├────────────────────────────────────────┼──────────────────────┼───────────────────┼─────────────────────────────┤
│ Schema Engine Rust binary │ uses native-tls │ uses native-tls │ macOS fails, Linux works │
├────────────────────────────────────────┼──────────────────────┼───────────────────┼─────────────────────────────┤
│ Prisma Client (with │ uses Node tls module │ same │ Works (Node defaults work) │
│ @prisma/adapter-pg) │ │ │ │
├────────────────────────────────────────┼──────────────────────┼───────────────────┼─────────────────────────────┤
│ Schema Engine on Linux │ n/a │ Works │ Works │
└────────────────────────────────────────┴──────────────────────┴───────────────────┴─────────────────────────────┘
Why the mismatch surfaces as bad protocol version
When the macOS Schema Engine negotiates TLS 1.0 with a PostgreSQL server requiring TLS 1.2+:
Code path
This is the single chokepoint for all PostgreSQL TLS connections via the native-tls backend in the Schema Engine.
────────────────────────────────────────────────────────────────────────────────
🛠 The Fix
In quaint/src/connector/postgres/native/mod.rs, configure TlsConnectorBuilder to enforce TLS 1.2 as the minimum
protocol version:
Why this is correct (not a workaround)
untouched
────────────────────────────────────────────────────────────────────────────────
✅ Verification
Compilation
Test results
All 333 / 333 total unit tests passed:
#[ignore] and only execute when a database is available, unrelated to the TLS fix and pre-existing baseline
behavior
Behavior validation
Test summary
┌──────────────────────────────────┬─────────────────────────────────────────────────────────┐
│ Test scope │ Result │
├──────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ Rust unit tests │ 333 / 333 passed │
├──────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ Integration tests (DB-dependent) │ Skipped (require live PostgreSQL) — pre-existing │
├──────────────────────────────────┼─────────────────────────────────────────────────────────┤
│ Pre-existing baseline │ Confirmed identical — TLS fix introduces no regressions │
└──────────────────────────────────┴─────────────────────────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────
📂 Files Modified
────────────────────────────────────────────────────────────────────────────────
🔗 Related
┌─────────────────────────────────────────────────────────────┬─────────────┬─────────────────────────────────────┐
│ Link │ Type │ Description │
├─────────────────────────────────────────────────────────────┼─────────────┼─────────────────────────────────────┤
│ prisma/prisma#29600 │ Resolves │ Original user report — failed TLS │
│ (prisma/prisma#29600) │ │ handshake on macOS │
├─────────────────────────────────────────────────────────────┼─────────────┼─────────────────────────────────────┤
│ prisma/prisma#28909 │ Related │ adapter-pg TLS error mapping fix │
│ (prisma/prisma#28909) │ │ │
├─────────────────────────────────────────────────────────────┼─────────────┼─────────────────────────────────────┤
│ prisma/prisma-engines │ Code │ Where the Schema Engine Rust code │
│ (https://github.com/prisma/prisma-engines) │ location │ lives │
├─────────────────────────────────────────────────────────────┼─────────────┼─────────────────────────────────────┤
│ Apple Security framework │ Reference │ Apple's Security framework TLS │
│ (https://developer.apple.com/documentation/security) │ │ protocol defaults │
└─────────────────────────────────────────────────────────────┴─────────────┴─────────────────────────────────────┘
────────────────────────────────────────────────────────────────────────────────
📋 Test Checklist
────────────────────────────────────────────────────────────────────────────────
Why not change Linux behavior?
A more aggressive fix could set TLS 1.2 minimum on all platforms, but this would:
The #[cfg(target_os = "macos")] guard is intentional and minimal-blast-radius.
Why no new integration test?
Integration tests for TLS protocol negotiation require:
This is best validated via:
Future improvements (out of scope for this PR)
────────────────────────────────────────────────────────────────────────────────
✅ Sign-off Criteria
────────────────────────────────────────────────────────────────────────────