fix(api): prevent Responses WebSockets from following redirects - #2513
Conversation
Castiron-Internal-PR: openai/openai-node-internal#87 Castiron-Source-SHA: 87734e0d859d6c35195ec7e0019a787fad7f12c6 Castiron-Public-Base-SHA: 222f3d7
Castiron custom code✅ No new custom-code files detected. 32 mixed files remain; 0 existing customizations changed. Compared 32 existing customizations unchanged
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 33195245898 --repo openai/openai-node \
--name castiron-custom-code-33195245898-1 --dir /tmp/castiron-custom-code-33195245898-1
git apply --stat /tmp/castiron-custom-code-33195245898-1/custom-code.patch
cat /tmp/castiron-custom-code-33195245898-1/custom-code.patchOr reproduce it from an SDK checkout containing the vendored reporter: git fetch --no-tags origin 222f3d7dffd9fd3d3d183473e1a7810e5e027d7e 1764410f356ac669fa109587e56dabb814f852b2
python3 scripts/castiron/custom_code_report.py report \
--base 222f3d7dffd9fd3d3d183473e1a7810e5e027d7e \
--head 1764410f356ac669fa109587e56dabb814f852b2 --fetch --require-head-hash --public \
--out /tmp/castiron-custom-code-1764410f356a
cat /tmp/castiron-custom-code-1764410f356a/custom-code.patchThis is the current full custom patch for mixed files, not an attribution of only the handwritten lines changed by this PR. |
apcha-oai
changed the title
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: 1764410f35
ℹ️ 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".
| ...authHeaders, | ||
| ...this._wsOptions?.headers, | ||
| }, | ||
| followRedirects: false, |
There was a problem hiding this comment.
Callers still see followRedirects as a supported field because ResponsesWSClientOptions extends WS.ClientOptions, but this override now silently ignores followRedirects: true on both stable and beta surfaces. Existing applications relying on same-origin or credential-free redirects will fail with the redirect response, while the canonical Responses WebSocket documentation merely says that WebSocket options can be passed through. Document this new invariant in the public JSDoc or canonical guide so the intentional breaking contract is discoverable.
AGENTS.md reference: AGENTS.md:L72-L74
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
expected for now; can followup if confusing
There was a problem hiding this comment.
context: WS.ClientOptions is node behavior so there's not a nice way to remove the key unless we exclude it or override with an inline deprecated marker.
this is blocking some things. Might do that separately to enforce followRedirects?: false only but punting for now to unblock downstream work
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Reviewed the exact current head, including the stable/beta option ordering, reconnect path, Node adapter behavior, redirect event/error behavior, Bedrock/no-auth coverage, and credential/noncredential redirect tests. followRedirects: false is applied after all caller options on every Responses socket creation, so caller credentials and request state cannot be replayed to a redirect target. No blocking security or correctness findings. The existing public-option documentation thread is a worthwhile follow-up but does not weaken this security boundary.
2 participants
Summary
Responses WebSockets now consistently refuse HTTP redirects, even when
followRedirects: trueis supplied by the caller. This closes the remaining path where credentials or other caller-provided request state could be replayed to a redirect destination.The first response is surfaced directly as an error (for example,
Unexpected server response: 302), noredirectevent is emitted, and no request reaches the destination. The initial request may still carry its configured credentials; the security boundary is that those credentials never leave the original endpoint.Implementation details
Both stable and beta Responses WebSockets apply
followRedirects: falseafter caller options. The redirect-security tests assert the unconditional policy, including benign-header and Bedrock cases. The packed-package source-navigation peer shim models the standardwsredirect option for TypeScript 4.9. Selective filtering used by other WebSocket surfaces remains unchanged.