feat(auth): integrate X.509 workload identity with the Ruby client - #525
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. 48 mixed files remain; 3 existing customizations changed. Compared
45 existing customizations unchanged
5 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 32812248210 --repo openai/openai-ruby \
--name castiron-custom-code-32812248210-1 --dir /tmp/castiron-custom-code-32812248210-1
git apply --stat /tmp/castiron-custom-code-32812248210-1/custom-code.patch
cat /tmp/castiron-custom-code-32812248210-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 5eb9eae682b495613823debb99dbc2fade62b5a9 35c118493d3723de9fc646e810dd49dc0aff9bda
python3 scripts/castiron/custom_code_report.py report \
--base 5eb9eae682b495613823debb99dbc2fade62b5a9 \
--head 35c118493d3723de9fc646e810dd49dc0aff9bda --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-35c118493d37
cat /tmp/castiron-custom-code-35c118493d37/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: bf02d0ae43
ℹ️ 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".
a8fe7fd to
246feac
Compare
bf02d0a to
07a7f32
Compare
246feac to
6620b48
Compare
07a7f32 to
962dcca
Compare
6620b48 to
cb8e3fa
Compare
962dcca to
ce99366
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce993664cc
ℹ️ 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".
ce99366 to
afccefa
Compare
cb8e3fa to
577f6d6
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: afccefa4b8
ℹ️ 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".
577f6d6 to
3bcda14
Compare
067b5b8 to
8858a0e
Compare
3bcda14 to
5430b4f
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8858a0eee8
ℹ️ 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".
5430b4f to
e1af164
Compare
8858a0e to
c9173dc
Compare
HAYDEN-OAI
left a comment
HAYDEN-OAI
left a comment
There was a problem hiding this comment.
Reviewed the standalone client integration, attested transport and issuer boundaries, endpoint/copy compatibility, security-scheme selection, request hooks, and stacked consumers. Three PR-attributable P2 findings are inline.
Inherited stack-level P2 (introduced in parent #524, not in this PR): the X.509 issuer returns nested OAuth errors such as {"error":{"code":"invalid_grant"}} and {"error":{"code":"invalid_subject_token"}}, but X509TokenExchange currently accepts only a flat error string. Consequently its OAuthError drops the documented machine-readable issuer code. Extract and allowlist the nested code while continuing to redact descriptions and other response details.
e1af164 to
c9801a6
Compare
c9173dc to
bd6a425
Compare
## Summary Fourth PR in the stacked X.509 workload-identity series; hardens the integrated flow while preserving existing JWT/ID workload-identity contracts. - Compare-and-invalidate only the actual rejected bearer; invalidate non-replayable requests and failed replays without replaying request bodies more than once. - Propagate the real shared-refresh failure to concurrent waiters and share one X.509 request deadline across refresh/replay without changing the existing per-retry JWT timeout behavior. - Validate the final destination and exact issued bearer **after** all request-preparation hooks, using the independent outer request as the trusted reference; preserve safe application hooks and separate admin credentials. - Reject hostile credential/header mutations before disclosure and redact cached bearer tokens and identity configuration from authenticator inspection. - Add deterministic lifecycle, concurrency, stale-generation, hostile-hook, mutable-original-request, safe-hook, and token-redaction regression tests. - Introduce a small general final-request validation seam in the existing base client, with matching Sorbet/RBS declarations. ## Verification - `bundle exec ruby test/openai/auth/x509_lifecycle_test.rb` - `bundle exec ruby test/openai/auth/workload_identity_test.rb` - `bundle exec ruby test/openai/resources/polling_helpers_test.rb --name test_workload_identity_timeout_uses_the_sdk_error_and_retry_contract` - `bundle exec rake test` - `bundle exec rake lint` - An independent security review reproduced the hostile-hook and mutable-original-request attacks before the fixes and confirmed both now fail before API dispatch. ## Stack 1. Attested transport and real-wire security boundary. 2. X.509 identity and pinned token-exchange protocol. 3. Public-client integration and two-leg mTLS end-to-end coverage. 4. **This PR:** lifecycle, retry, concurrency, and final-credential hardening. 5. Customer documentation, enrolled-credential live smoke, and installed-gem end-to-end coverage. Review requested from `@openai/sdks-team` because this change affects authentication, bearer handling, and request preparation.
…09-preserve-merged-stack # Conflicts: # lib/openai/auth/x509_token_exchange.rb # test/openai/auth/x509_token_exchange_test.rb
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 275ca73392
ℹ️ 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: 39f578cf58
ℹ️ 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: 04e4f86734
ℹ️ 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: 066ddef1ac
ℹ️ 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 complete updated client/transport integration, selected authentication schemes, immutable post-preparation bearer validation, bounded shared issuer/API retries and Retry-After handling, concurrency, cancellation, copy/residency behavior, response cleanup, and prior discussions. All three earlier inline findings are fixed; one remaining inherited issuer-protocol defect becomes production-visible through this PR's public integration and is identified inline.
… coverage (#527) ## Summary Final PR in the stacked X.509 workload-identity series. Adds customer-facing setup guidance, installed-gem verification, and optional live smoke coverage for both ordinary SDK calls and enrolled X.509 identities. - Document the preview API, application-owned certificate/key material, lack of cryptographic bearer binding, supported mTLS origins, strict direct/HTTP-CONNECT proxy modes, and unsupported providers/Realtime transports. - Add executable `examples/x509_workload_identity.rb`, which performs a real certificate-authenticated token exchange and mTLS API request using enrolled local credentials. - Build and install the actual gem into an isolated `GEM_HOME`, then verify a genuine two-leg mTLS/HTTP-CONNECT flow against ephemeral local issuer/API servers. - Add optional local `bundle exec rake test:live:smoke` coverage for authenticated model listing, a normal Responses API request, and a completed streaming response; `OPENAI_LIVE_SMOKE_X509=1` additionally exercises the enrolled-certificate example. - Add a `workflow_dispatch`-only **Live Smoke** workflow. Ordinary API smoke uses protected `ci`; optional X.509 smoke runs in a separate dependent job protected by the dedicated `x509-live-smoke` environment, which disables administrator bypass, prevents self-review, requires independent SDK-team approval, and permits protected branches only. - Keep ordinary API-key and X.509 credentials in different jobs and different protected environments. X.509 PEM material is written only to mode-restricted ephemeral runner files, removed from the child environment, cleaned up on exit, and never uploaded as an artifact. - Add deterministic runner/workflow security regressions; no live workflow is a required pull-request check. ## Verification - Full Ruby 4 suite: **1,338 tests, 11,973 assertions, zero failures/errors**; Ruby 3.3 also passed all **1,338 tests**. - Focused X.509, packaging, and protected-workflow suites passed independently on Ruby 3.3, 3.4, and 4.0. - `bundle exec rake lint`: **2,764 files, zero offenses**. - `python3 -B -m unittest discover -s scripts/castiron -p 'test_custom_code*.py'`. - Real production X.509 issuer exchange and mTLS API requests succeeded separately on Ruby 3.3, 3.4, and 4.0 using the enrolled restricted project service account. - The dedicated `x509-live-smoke` environment has administrator bypass disabled, independent SDK-team approval enabled, and all four required X.509 secrets configured; those secrets have been removed from shared `ci`. The offline end-to-end flow performs real certificate-authenticated TLS handshakes and authenticated CONNECT without live credentials. Production certificate material, issuer tokens, private account identifiers, and API keys are never committed, logged, or required by ordinary CI. ## Stack 1. Attested transport and real-wire security boundary. 2. X.509 identity and pinned token-exchange protocol. 3. Public-client integration and two-leg mTLS end-to-end coverage. 4. Refresh/retry/concurrency and final-credential hardening. 5. **This PR:** documentation, installed-gem end-to-end coverage, enrolled-certificate smoke checks, and optional protected whole-library live smoke testing. Review requested from `@openai/sdks-team` because this PR adds certificate-authenticated credential documentation and a security-sensitive GitHub Actions workflow.
…09-client-path-followup
apcha-oai
left a comment
apcha-oai
left a comment
There was a problem hiding this comment.
[sdk codex review] Approved at 35c118493d3723de9fc646e810dd49dc0aff9bda after two independent full reviews of all 24 changed files and the supporting client/authentication contracts. No substantiated merge-blocking defect found.
Reviewed final credential/security-scheme validation, cache and refresh-generation synchronization, stale-token invalidation, one-time 401 replay, shared issuer/API retry budgets, unread non-replayable bodies, endpoint/residency/copy compatibility, legacy JWT/API-key/admin behavior, signatures, packaging, documentation and the optional live workflow. The existing findings, including the nested OAuth-code fix in parent #523, were checked in source; no duplicate comments added.
Replacement CI 32812201205 passed 1,340 tests on each of Ruby 3.3.12, 3.4.10 and 4.0.6, with zero failures/errors/skips. Packaging and signature checks also passed. These jobs tested merge 9e24b5f28377de6a6c4819d7ac4d048513af3688, this head plus target parent 5eb9eae682b495613823debb99dbc2fade62b5a9, not a separate merge with default main. The failed aggregator on older run 32812200629 had canceled dependencies; the same-head/parent replacement completed successfully.
The actual x509-live-smoke environment requires independent SDK-team approval, prevents self-review and disables administrator bypass. The workflow remains optional, main-only and least-privilege. No live-smoke execution or production enrollment is claimed.
Head, target/actual/live parent, author, open/non-draft state, mergeability and feedback were checked before submission. No candidate code, tests, builds, live API calls or security scans were run; separately required human/owner sign-off remains separate.
69dc4af
into
codex/x509-ruby-1-attested-transport
3 participants
Summary
Final open integration PR in the stacked X.509 workload-identity series. The lifecycle-hardening PR (#526) and documentation/live-smoke PR (#527) have already been merged into this branch, so this PR now contains their complete reviewed changes as well.
OpenAI::Auth::X509WorkloadIdentitythrough the existingworkload_identity:client option and require an attestedX509Transportashttp_client:.Verification
bundle exec rake lint: 2,765 files inspected, no offenses.Stack
Review requested from
@openai/sdks-teambecause this change affects client authentication, endpoints, TLS, and a protected CI workflow.