fix(realtime): validate session event variants - #733
markstuart-oai wants to merge 1 commit into
Conversation
Castiron custom code✅ No new custom-code files detected. 70 mixed files remain; 0 existing customizations changed. Compared 70 existing customizations unchanged
30 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 35663967240 --repo openai/openai-ruby \
--name castiron-custom-code-35663967240-1 --dir /tmp/castiron-custom-code-35663967240-1
git apply --stat /tmp/castiron-custom-code-35663967240-1/custom-code.patch
cat /tmp/castiron-custom-code-35663967240-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin ff88b28e050dffd71ef983b32b1d4d54329c00dd 0590906aef15091b3f6fcdb3413c4e7424da96fd
python3 scripts/castiron/custom_code_report.py report \
--base ff88b28e050dffd71ef983b32b1d4d54329c00dd \
--head 0590906aef15091b3f6fcdb3413c4e7424da96fd --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-0590906aef15
cat /tmp/castiron-custom-code-0590906aef15/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
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. |
![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: 04329c00aa
ℹ️ 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".
| raise ArgumentError, "Realtime session must be an object with a string type" | ||
| end | ||
|
|
||
| validate_discriminator!(session, discriminator_values(session_union), kind: "session") |
There was a problem hiding this comment.
When a custom or proxied Realtime endpoint returns an unsupported session.type containing sensitive text, this call reaches validate_discriminator!, whose ArgumentError interpolates the raw value; the rescue then preserves it as the RealtimeProtocolError cause. Although error.message is sanitized, Ruby's Exception#full_message and standard exception logging include cause messages, so the session value is exposed—the new test only checks the outer message. Raise a server-side validation cause that does not include the received discriminator.
AGENTS.md reference: AGENTS.md:L43-L46
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
The existing fix at 0590906aef15091b3f6fcdb3413c4e7424da96fd addresses this: the server-side check now raises the constant ArgumentError, "Unsupported Realtime session type" rather than calling the value-interpolating validator. The regression checks error.message, error.full_message, and error.cause.message for the synthetic unsupported value. Client-side validation is unchanged.
I re-inspected the complete patch and the passing current-head CI, including Ruby 3.3/3.4/4.0. I am not claiming a new local test pass: the local rerun could not start because pinned gems were unavailable. The later human approvals remain recorded; I have not dismissed a review. This outdated thread is left open for reviewer clearance, and the PR currently has a separate merge conflict.
dpiet-oai
left a comment
dpiet-oai
left a comment
There was a problem hiding this comment.
[High] Redact unsupported nested session tags from the exception cause
lib/openai/helpers/realtime/connection.rb:55
The unresolved current-head finding at #733 (comment) is verified and merge-blocking. validate_discriminator! interpolates the server-provided session.type into an ArgumentError, and parse_event retains it as the RealtimeProtocolError cause. A custom or proxied endpoint can therefore place sensitive response content in that field and have Exception#full_message or standard exception logging emit it, despite the sanitized outer message.
Suggested fix: Raise a generic server-side discriminator validation cause that omits the received value, and cover the cause or full_message in the regression test.
Castiron-Internal-PR: openai/openai-ruby-internal#133 Castiron-Source-SHA: bbb68202ab97d3f04b7060a8f00612ddee51e9a8 Castiron-Public-Base-SHA: 3343547
04329c0 to
0590906
Compare
|
Responding to the redaction change request: the published fix uses a constant server-side validation cause, with regression assertions for both the cause and full exception message. I added the details directly to the original inline thread. Thanks for the subsequent approvals. The current review decision is APPROVED, and current-head CI passes, but GitHub currently reports a merge conflict against |
2 participants
Summary
Use the session type to select realtime or transcription models in session creation and update events, and reject missing or unsupported nested session types in WebSocket connections.
Changes
Known session creation/update events now require a supported nested session type. Future unsupported nested session kinds raise RealtimeProtocolError until the SDK supports them.
Source monorepo PR: #1533980.