feat(client): add native mTLS transport recipe - #828
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds documentation and runnable/tested examples showing how to use mutual TLS (mTLS) with the OkHttp-based OpenAI Java SDK via native JSSE (SSLContext/KeyStore), and exposes followRedirects(boolean) to Java callers to help prevent client-certificate disclosure to redirect targets.
Changes:
- Documented an end-to-end “native JSSE mTLS” setup recipe in
README.md, including endpoint selection and redirect guidance. - Added a compilable
MutualTlsExampleJava example demonstrating PKCS#12 loading and JSSE + SDK wiring. - Added hermetic tests verifying full client certificate chains are presented and that missing intermediates fail closed; added OkHttp test dependencies.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Adds the documented “native JSSE mTLS” recipe and links to the beta opt-in guide. |
| openai-java-example/src/main/java/com/openai/example/MutualTlsExample.java | Adds a runnable Java example for configuring mTLS via JSSE and SDK OkHttp hooks. |
| openai-java-client-okhttp/src/test/kotlin/com/openai/client/okhttp/OpenAIOkHttpClientNativeMutualTlsTest.kt | Adds hermetic MockWebServer-based tests for full-chain mTLS and missing-intermediate failures. |
| openai-java-client-okhttp/src/main/kotlin/com/openai/client/okhttp/OpenAIOkHttpClientAsync.kt | Exposes followRedirects(boolean) to Java (removes @JvmSynthetic) and documents default. |
| openai-java-client-okhttp/src/main/kotlin/com/openai/client/okhttp/OpenAIOkHttpClient.kt | Exposes followRedirects(boolean) to Java (removes @JvmSynthetic) and documents default. |
| openai-java-client-okhttp/build.gradle.kts | Adds mockwebserver and okhttp-tls test dependencies for the new mTLS tests. |
Suppressed comments (1)
README.md:1734
- This path construction will throw a NullPointerException if
OPENAI_MTLS_KEYSTOREis unset. Using an explicit null check (orObjects.requireNonNull) makes the sample fail with a clearer message.
Files.newInputStream(Paths.get(System.getenv("OPENAI_MTLS_KEYSTORE")))) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed exact head 98eb8d9. The native JSSE/OkHttp transport integration, default server trust and hostname verification, redirect disabling, rotation guidance, and Java runtime compatibility are sound, but the endpoint-selection, cross-provider credential, and certificate-chain enablement issues called out inline should be addressed before merge.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (2)
README.md:1740
- Same eager-evaluation issue as the API key:
System.getenv("OPENAI_BASE_URL")is read even whenopenai.baseUrlis set, which can break the recipe in restricted environments and is inconsistent with the SDK’sSystem.getProperty(...) ?: System.getenv(...)precedence style.
String baseUrl = System.getProperty("openai.baseUrl", System.getenv("OPENAI_BASE_URL"));
if (baseUrl == null || baseUrl.isEmpty()) {
baseUrl = "https://mtls.api.openai.com/v1";
}
README.md:1736
- In this snippet,
System.getenv("OPENAI_API_KEY")is evaluated eagerly as the default argument toSystem.getProperty(...), even whenopenai.apiKeyis present. This can cause the recipe to fail in environments where env access is restricted (e.g., SecurityManager / sandboxing) despite a correctly set system property. It also diverges from the SDK’s established precedence pattern (System.getProperty(...) ?: System.getenv(...), e.g.openai-java-core/src/main/kotlin/com/openai/core/ClientOptions.kt:585-590).
This issue also appears on line 1737 of the same file.
String apiKey = System.getProperty("openai.apiKey", System.getenv("OPENAI_API_KEY"));
if (apiKey == null || apiKey.isEmpty()) {
throw new IllegalStateException(
"openai.apiKey or OPENAI_API_KEY must be set for OpenAI mTLS");
}
254c928 to
e876c5c
Compare
|
Addressed the two suppressed Copilot findings in |
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed exact head e876c5c. The three findings from review 4832028958 are resolved: the recipes explicitly require an OpenAI bearer key, preserve configured EU/custom endpoints, and document certificate-chain enablement. One substantive project/organization-scope regression remains, described inline.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b2a895688
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 1fb0ebb. The prior credential-selection, EU/custom endpoint, certificate-chain, organization/project-scope, and empty-endpoint findings are fixed. One newly validated P1 remains: a configured HTTP endpoint bypasses TLS/client authentication and sends the OpenAI bearer key in plaintext; details inline.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (3)
openai-java-client-okhttp/src/test/kotlin/com/openai/client/okhttp/OpenAIOkHttpClientNativeMutualTlsTest.kt:36
- The test name suggests it validates that the full PKCS#12 chain is presented, but the test only asserts that a request succeeds when the intermediate is included. Renaming the test to reflect what it actually verifies will make failures easier to interpret.
fun nativeMutualTlsPresentsFullPkcs12Chain() {
openai-java-example/src/main/java/com/openai/example/MutualTlsExample.java:119
requireHttpsBaseUrldiscards theIllegalArgumentExceptioncause when parsing the URI fails, which makes it harder to diagnose invalid URLs. Preserve the original exception as the cause.
try {
baseUri = URI.create(baseUrl);
} catch (IllegalArgumentException ignored) {
throw new IllegalStateException("OpenAI mTLS requires a valid HTTPS base URL");
}
README.md:1756
- The README recipe discards the
IllegalArgumentExceptioncause when parsingbaseUrlfails, which makes debugging misconfigured URLs harder. Preserve the original exception as the cause.
try {
baseUri = URI.create(baseUrl);
} catch (IllegalArgumentException ignored) {
throw new IllegalStateException("OpenAI mTLS requires a valid HTTPS base URL");
}
|
Addressed the latest suppressed Copilot feedback in |
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Re-reviewed exact head 00dcd3d. All prior findings are resolved: explicit OpenAI bearer-key selection, EU/custom endpoint and organization/project scope preservation, fail-closed empty/non-HTTPS endpoint handling, redirect suppression, and complete client-certificate-chain documentation plus presented-chain verification. No substantive issues found.
Summary
followRedirects(boolean)to Java callers so mTLS clients can avoid presenting their client identity to redirect targetsDesign
This intentionally does not add a first-class certificate model or mTLS state to the SDK. Callers build a
KeyStore,KeyManagerFactory, andSSLContextwith native JSSE, then pass the resulting socket factory and the separately configured server trust manager through the existing OkHttp builder hooks.The caller explicitly selects the global, EU, or custom mTLS endpoint. Certificate rotation rebuilds the SSL context, HTTP transport, and SDK client. API-key authentication remains required; certificate-only WIF and Realtime/WebSocket support are outside this change.
Validation
./gradlew :openai-java-client-okhttp:test --tests com.openai.client.okhttp.OpenAIOkHttpClientNativeMutualTlsTest :openai-java-example:compileJava./gradlew lint :openai-java-client-okhttp:test :openai-java-example:compileJavagit diff --check