feat(auth): add X.509 token exchange - #940
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Castiron custom code✅ No new custom-code files detected. 53 mixed files remain; 0 existing customizations changed. Compared 53 existing customizations unchanged
13 more in the full report. A changed generated baseline means this report cannot reliably identify which handwritten lines changed. Inspect the custom-code diffDownload the exact patch produced by this run (requires repository access): gh run download 33089784911 --repo openai/openai-java \
--name castiron-custom-code-33089784911-1 --dir /tmp/castiron-custom-code-33089784911-1
git apply --stat /tmp/castiron-custom-code-33089784911-1/custom-code.patch
cat /tmp/castiron-custom-code-33089784911-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 98e1d21b1c91d3c12f138a471afa78d1d8ebf0e9 8484d16167dbd2c74a7aa012e45f37d878a4105f
python3 scripts/castiron/custom_code_report.py report \
--base 98e1d21b1c91d3c12f138a471afa78d1d8ebf0e9 \
--head 8484d16167dbd2c74a7aa012e45f37d878a4105f --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-8484d16167db
cat /tmp/castiron-custom-code-8484d16167db/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
![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: 75dfa8f281
ℹ️ 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.
Reviewed the fixed mTLS issuer boundary, request and token validation, sync/async cancellation ownership, OkHttp response handoff, and existing review threads. The unresolved inline on responseReader.readTree remains substantive: it materializes the entire unbounded issuer response, including arbitrarily large forward-compatible fields, independently in up to four concurrent parser threads; extract the required token fields incrementally while retaining duplicate/trailing-token validation and supported large payloads. The separate existing inline on OAuth errors is also valid: unlike the established workload-identity exchange, non-200 responses lose sanitized error/error_description diagnostics and return only an Unknown error; preserve safe, redacted OAuth diagnostics without exposing credentials. Referencing the existing unresolved threads rather than duplicating their inline comments.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f29339ab38
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9da1d2f25e
ℹ️ 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 the full X.509 exchange after the earlier feedback. Confirmed issuer responses now use incremental duplicate-safe JSON parsing with pre-materialization bounds, OAuth diagnostics preserve only safe allowlisted codes while redacting credential-bearing text and headers, and I/O causes remain retryable. Also verified fixed HTTPS mTLS issuer routing with redirects disabled, exact grant and issued-token validation, positive token lifetimes, bounded four-active/four-queued async admission, cancellation-safe response ownership, and sync/async parity. Previous substantive concerns are resolved.
2 participants
Summary
X509TransportfoundationIOExceptionandSocketTimeoutExceptioncauses as retryable I/O failuresScope
This is the bounded token-exchange layer only. It does not add caching or refresh, client-builder integration, public API, generated service changes, generic retry changes, Castiron changes, or broader lifecycle/deadline/cancellation architecture.
Validation
./scripts/test./scripts/lint./scripts/detect-breaking-changes 35324bf54adf68ea1853f7684bdbdf29c9fd329cfc7c6c43-570b-4fa3-944a-91f9e2d6d721: complete coverage, no findings