fix(desktop): trust OS root certificates (Watt Toolkit / corporate MITM)#516
fix(desktop): trust OS root certificates (Watt Toolkit / corporate MITM)#516rainxchzed merged 2 commits intomainfrom
Conversation
…corporate MITM proxies work
WalkthroughThe PR adds support for JVM platforms to trust OS-installed root certificates by integrating a composite TLS trust manager that combines platform defaults with OS-specific certificate stores (Windows-ROOT on Windows, KeychainStore on macOS). Release notes are updated across 14 localized versions documenting this feature fix. ChangesOS Trust Chain Integration
Release Notes Localization
Sequence DiagramsequenceDiagram
actor HC as HTTP Client<br/>Factory
participant OTM as OS Trust<br/>Manager
participant OSK as OS Keystore<br/>Detection
participant DTM as Platform<br/>Default TM
participant CTM as Composite<br/>TM
participant SSL as SSL<br/>Context
HC->>OTM: buildOsTrustChainOrNull()
OTM->>OSK: Detect OS (Windows/macOS)<br/>Select Keystore Type
OSK-->>OTM: Keystore Type
par Load Trust Managers
OTM->>DTM: Load platform default
OTM->>OSK: Load OS-specific from<br/>selected keystore
end
DTM-->>OTM: Default TM
OSK-->>OTM: OS TM (if available)
alt Two or more TMs available
OTM->>CTM: Create CompositeX509TrustManager
CTM->>SSL: Initialize SSLContext('TLS')<br/>with composite
SSL-->>OTM: Return (SSLSocketFactory,<br/>CompositeX509TrustManager)
OTM-->>HC: OsTrustChain
else Fewer than 2 TMs
OTM-->>HC: null
end
HC->>HC: Apply to OkHttp engine config
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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.
🧹 Nitpick comments (3)
core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/OsTrustManager.kt (2)
27-29: ⚡ Quick winNarrow
catch (_: Throwable)and surface failures.Catching
ThrowableswallowsOutOfMemoryError,InterruptedException(if it ever propagates asThrowableon this path) and ordinary programming bugs. In particular, incheckClientTrusted/checkServerTrustedyou record any unchecked exception orErroraslastErrorand then, if every delegate fails, rethrow thatThrowablefrom a method whose contract isthrows CertificateException— a JSSE caller assertingcatch (e: CertificateException)will not catch a wrappedRuntimeExceptionand the failure mode becomes confusing.Two suggestions:
- Narrow each
catchtoException(orCertificateExceptionin the per-delegate trust loops) so genuine VM errors propagate.- Wrap
lastErrorasCertificateException(cause = lastError)when it isn't already aCertificateException, so the method stays contract-faithful.A debug log (even at
Logger.getLogger(...).fine(...)) on the swallow path would also save a lot of time the next time someone debugs why the OS chain "silently didn't load."♻️ Proposed adjustment for the trust loops
override fun checkServerTrusted(chain: Array<out X509Certificate>, authType: String) { - var lastError: Throwable? = null + var lastError: CertificateException? = null for (tm in delegates) { try { tm.checkServerTrusted(chain, authType) return - } catch (t: Throwable) { - lastError = t + } catch (e: java.security.cert.CertificateException) { + lastError = e } } throw lastError ?: java.security.cert.CertificateException("No trust manager accepted the chain") }Also applies to: 44-48, 50-56, 67-69, 80-82
🤖 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 `@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/OsTrustManager.kt` around lines 27 - 29, In OsTrustManager.kt narrow the overly broad `catch (_: Throwable)` blocks in the `checkClientTrusted` and `checkServerTrusted` delegate loops to catch `Exception` (or specifically `CertificateException` for per-delegate failures) so Errors like OOME/AssertionError propagate; when all delegates fail, if `lastError` is not already a `CertificateException` wrap it as `CertificateException(cause = lastError)` before rethrowing to preserve the method contract; also add a fine/debug log on the swallow path (e.g. via Logger.getLogger(...) .fine(...)) to record each delegated failure for diagnostics.
58-89: 🏗️ Heavy liftConsider implementing
X509ExtendedTrustManagerif delegate managers use it.
CompositeX509TrustManageronly implements the baseX509TrustManagerinterface. If any delegate is anX509ExtendedTrustManager, its socket/SSLEngine-aware overloads (which support endpoint identification during the SSL handshake) will not be invoked. For consistency with TLS best practices, the composite should implementX509ExtendedTrustManagerand forward to the extended overloads when available, falling back to the legacy overloads otherwise.This is a technical refinement for defense-in-depth on endpoint verification. OkHttp's hostname verification (via
HostnameVerifier) still applies independently and will catch certificate mismatches; this change would ensure the SSL handshake layer also validates endpoint identity when the underlying trust managers support it.🤖 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 `@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/OsTrustManager.kt` around lines 58 - 89, CompositeX509TrustManager currently implements X509TrustManager only; update it to implement X509ExtendedTrustManager and add overrides for the socket/SSLEngine-aware methods (checkClientTrusted(chain, authType, Socket) and checkClientTrusted(chain, authType, SSLEngine), and similarly for checkServerTrusted) that forward to each delegate: if a delegate is an X509ExtendedTrustManager call its extended method, otherwise fall back to the legacy checkClientTrusted/checkServerTrusted; preserve the existing behavior of returning on first success and collecting the last exception to throw, and keep getAcceptedIssuers() as-is; add necessary imports for X509ExtendedTrustManager, Socket, and SSLEngine.core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.jvm.kt (1)
17-25: 💤 Low valueConsider caching the OS trust chain across client builds.
buildOsTrustChainOrNull()performs OS-keystore I/O (loadingWindows-ROOT/KeychainStore, building aTrustManagerFactory, initializing anSSLContext) on every call tocreatePlatformHttpClient. Each proxy setting toggle re-pays that cost, and OkHttp/Conscrypt actually prefer a single sharedSSLSocketFactoryinstance per process for connection-pool reuse.A top-level
private val osTrustChain by lazy { buildOsTrustChainOrNull() }(or a memoized accessor inOsTrustManager.kt) would address both at once. The TLS configuration here doesn't change perProxyConfig, so it's safe to share.♻️ Sketch
+private val osTrustChain: OsTrustChain? by lazy { buildOsTrustChainOrNull() } + actual fun createPlatformHttpClient(proxyConfig: ProxyConfig): HttpClient = HttpClient(OkHttp) { engine { config { - buildOsTrustChainOrNull()?.let { chain -> + osTrustChain?.let { chain -> sslSocketFactory(chain.socketFactory, chain.trustManager) }🤖 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 `@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.jvm.kt` around lines 17 - 25, createPlatformHttpClient currently calls buildOsTrustChainOrNull() on every client build which repeats OS keystore I/O and prevents sharing of the SSLSocketFactory; change this to cache the result (e.g. a top-level private val osTrustChain by lazy { buildOsTrustChainOrNull() } or a memoized accessor in OsTrustManager) and then use that cached osTrustChain in createPlatformHttpClient when calling sslSocketFactory(chain.socketFactory, chain.trustManager); ensure the cached value remains nullable and preserved across ProxyConfig-driven rebuilds so TLS configuration is shared process-wide.
🤖 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.
Nitpick comments:
In
`@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.jvm.kt`:
- Around line 17-25: createPlatformHttpClient currently calls
buildOsTrustChainOrNull() on every client build which repeats OS keystore I/O
and prevents sharing of the SSLSocketFactory; change this to cache the result
(e.g. a top-level private val osTrustChain by lazy { buildOsTrustChainOrNull() }
or a memoized accessor in OsTrustManager) and then use that cached osTrustChain
in createPlatformHttpClient when calling sslSocketFactory(chain.socketFactory,
chain.trustManager); ensure the cached value remains nullable and preserved
across ProxyConfig-driven rebuilds so TLS configuration is shared process-wide.
In
`@core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/OsTrustManager.kt`:
- Around line 27-29: In OsTrustManager.kt narrow the overly broad `catch (_:
Throwable)` blocks in the `checkClientTrusted` and `checkServerTrusted` delegate
loops to catch `Exception` (or specifically `CertificateException` for
per-delegate failures) so Errors like OOME/AssertionError propagate; when all
delegates fail, if `lastError` is not already a `CertificateException` wrap it
as `CertificateException(cause = lastError)` before rethrowing to preserve the
method contract; also add a fine/debug log on the swallow path (e.g. via
Logger.getLogger(...) .fine(...)) to record each delegated failure for
diagnostics.
- Around line 58-89: CompositeX509TrustManager currently implements
X509TrustManager only; update it to implement X509ExtendedTrustManager and add
overrides for the socket/SSLEngine-aware methods (checkClientTrusted(chain,
authType, Socket) and checkClientTrusted(chain, authType, SSLEngine), and
similarly for checkServerTrusted) that forward to each delegate: if a delegate
is an X509ExtendedTrustManager call its extended method, otherwise fall back to
the legacy checkClientTrusted/checkServerTrusted; preserve the existing behavior
of returning on first success and collecting the last exception to throw, and
keep getAcceptedIssuers() as-is; add necessary imports for
X509ExtendedTrustManager, Socket, and SSLEngine.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2ae0aa08-4bec-4976-aae2-c5e9c3ea7d9a
📒 Files selected for processing (15)
core/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.jvm.ktcore/data/src/jvmMain/kotlin/zed/rainxch/core/data/network/OsTrustManager.ktcore/presentation/src/commonMain/composeResources/files/whatsnew/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ar/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/bn/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/es/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/fr/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/hi/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/it/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ja/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ko/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/pl/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/ru/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/tr/16.jsoncore/presentation/src/commonMain/composeResources/files/whatsnew/zh-CN/16.json
1 participant
Closes #460
Summary
Desktop builds now trust certificates installed in the OS root store (Windows-ROOT keychain, macOS Keychain) in addition to the JVM-bundled
cacerts. Lets users behind TLS-intercepting tools — Watt Toolkit, FastGithub, Fiddler, corporate MITM proxies — keep using the app without manually injecting CAs into the JDK withkeytool.Why
Watt Toolkit (and similar China-side accelerators) terminate TLS with their own self-signed CA. The browser works because the installer adds that CA to the Windows cert store. Our desktop client uses the JVM cacerts bundle instead — Watt's CA isn't there, every github.com request 502s on TLS handshake, even with the app's "no proxy" mode (Watt's hosts-file mode is system-wide).
Implementation
OsTrustManager.kt(jvmMain) builds aCompositeX509TrustManagerthat delegates to the default JVM trust manager and the platform-native one (Windows-ROOTon Windows,KeychainStoreon macOS). Either accepts → handshake succeeds.HttpClientFactory.jvm.ktwires the composite into OkHttp viasslSocketFactory(...). Silently skipped on platforms where no OS keystore is available — default trust still applies./etc/ssl/certsinto cacerts at install time. Users on minimal distros can stillkeytool -importcert.Test plan
mkcert): app trusts intercepted github.com TLS.Summary by CodeRabbit
Bug Fixes
Documentation