fix: preserve explicit security-header removal - #1036
Merged
Merged
Conversation
Contributor
Castiron custom code✅ No new custom-code files detected. 83 mixed files remain; 0 existing customizations changed. Compared 83 existing customizations unchanged
43 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 35631524343 --repo openai/openai-java \
--name castiron-custom-code-35631524343-1 --dir /tmp/castiron-custom-code-35631524343-1
git apply --stat /tmp/castiron-custom-code-35631524343-1/custom-code.patch
cat /tmp/castiron-custom-code-35631524343-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 6087908a3d5e38f5b684c9bcba46bec162bbbed0 4cf6c1d2cdb566972e7f9092574b0e9ee1aae640
python3 scripts/castiron/custom_code_report.py report \
--base 6087908a3d5e38f5b684c9bcba46bec162bbbed0 \
--head 4cf6c1d2cdb566972e7f9092574b0e9ee1aae640 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-4cf6c1d2cdb5
cat /tmp/castiron-custom-code-4cf6c1d2cdb5/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
dpiet-oai
force-pushed
the
castiron/promotions/pr-92
branch
from
September 20, 2026 02:40
57b740c to
e44fb28
Compare
dpiet-oai
marked this pull request as ready for review
September 20, 2026 02:41
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. |
Contributor
dpiet-oai
force-pushed
the
castiron/promotions/pr-92
branch
from
September 21, 2026 15:51
e44fb28 to
f141b60
Compare
markstuart-oai
approved these changes
Sep 21, 2026
markstuart-oai
left a comment
markstuart-oai
left a comment
Contributor
There was a problem hiding this comment.
Reviewed f141b60. The copied removal set consistently suppresses generated API/admin/Azure headers and generic workload authentication while preserving explicit replacement headers, the original client, and auth-resource closure. The focused regressions cover HTTP and WebSocket paths, including throwing token suppliers and rejected upgrades. No actionable correctness or structural issues found.
Validation: inspected the pinned source, surrounding header composition and authentication paths, tests, and passing exact-head hosted checks (tests, lint, build, Java/Jackson compatibility and API compatibility). I did not independently execute tests.
Castiron-Internal-PR: openai/openai-java-internal#92 Castiron-Source-SHA: b417859f0f001896cdd47fe7b74c115a3976a369 Castiron-Public-Base-SHA: 6087908
dpiet-oai
force-pushed
the
castiron/promotions/pr-92
branch
from
September 21, 2026 17:21
f141b60 to
4cf6c1d
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explicit credential-header removal was lost during request preparation, allowing generated credentials to be reinserted. Preserve case-insensitive removal through copied client options and API/admin/Azure preparation, honor explicit replacements, and suppress generic workload authorization on both HTTP requests and Responses WebSocket connections. Removed bearer-token suppliers are skipped, including on Azure routes. Removed workload credentials are neither fetched nor invalidated after an unrelated rejected WebSocket upgrade. Original clients retain authentication and existing auth-resource close ownership.
Validation: 26 focused header-removal/workload tests and Kotlin lint pass. Public HTTP requests cover synchronous/asynchronous removal and replacement. Four public Responses connection cases cover removal/replacement with successful and rejected upgrades; all four fail on the previous implementation. Eight public request cases also verify that a throwing bearer-token supplier is skipped for ordinary/Azure routes, synchronous/asynchronous calls, and removal/replacement. Tests also verify original-client authentication and credential ownership.
This change covers generated API/admin/Azure credentials and generic workload authentication. Separately bound X.509 and Bedrock authentication is unchanged.