fix(security): redact invalid Azure authentication credentials - #2421
fix(security): redact invalid Azure authentication credentials#2421HAYDEN-OAI wants to merge 16 commits into
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5e2b22a09c
ℹ️ 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".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Independent two-pass security review confirms both existing unresolved exact-head P1 discussions; no duplicate inline comments are needed. Stable and beta Azure Realtime WebSocket factories bypass the new validator, and native WebSocket errors disclose malformed API-key or Entra bearer credentials. Supported request/default authentication-header overrides also bypass configured-key validation and disclose their effective secret; conversely invalid unused configured keys are rejected despite valid overrides. Validate the effective credential at the actual HTTP and WebSocket header-construction boundaries while preserving override precedence.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 222e643aa2
ℹ️ 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: 7ff52b636c
ℹ️ 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: eba9440c14
ℹ️ 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: 953b385759
ℹ️ 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".
Castiron custom code
The report setup or validation failed. Inspect the workflow run. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c006e3bcfe
ℹ️ 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".
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Independent two-pass review confirms Azure credential-redaction fixes, but a newly introduced iterator typing regression deterministically breaks both ecosystem CI and the breaking-change detector with TS2352.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 63f427f025
ℹ️ 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: 8ea03dc2d2
ℹ️ 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: 1cecd41ce0
ℹ️ 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".
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7f1728cf94
ℹ️ 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: 2a85796834
ℹ️ 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: 62f306e214
ℹ️ 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: c3002079da
ℹ️ 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: 59af3c0ce2
ℹ️ 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".
| ); | ||
|
|
||
| function hasIntrinsicHeadersIdentity(headers: RequestInit['headers']): headers is Headers { | ||
| if (!(headers instanceof Headers) || Object.getPrototypeOf(headers) !== intrinsicHeadersPrototype) { |
There was a problem hiding this comment.
Preserve unmodified Headers subclass identity
When prepareRequest installs an unmodified Headers subclass (for example, class TrackedHeaders extends Headers {}) whose identity carries WeakMap metadata for a custom fetch transport, this exact-prototype check rejects it and fetchWithAuth replaces it with a new Headers. The base revision forwarded that hook-supplied instance unchanged, and the remaining case after preserving exact native instances does not involve overridden header operations, so it can be validated safely without breaking the protected-hook identity contract.
AGENTS.md reference: AGENTS.md:L52-L57
Useful? React with 👍 / 👎.
| const { body, headers } = options; | ||
| const preprocessesHeaders = body === undefined ? 'body' in options : Boolean(body); | ||
| const protection = preprocessesHeaders ? protectAzureRequestHeaders(headers) : undefined; |
There was a problem hiding this comment.
Snapshot the effective headers property only once
When direct client.request() options expose headers through an accessor that returns different records on successive reads, this destructuring registers protection for the first record, but super.buildRequest() immediately reads the property again while making its shallow copy. For a truthy body, that second record is passed through the preliminary buildHeaders([rawHeaders]) without the marker; if it contains a credential such as secret\nsuffix, native Headers.append throws a diagnostic containing the secret before the protected final merge. Fresh evidence beyond the covered getters inside a header record is that the FinalRequestOptions.headers property itself can change between these reads, so the body and final merge must consume one captured representation.
AGENTS.md reference: AGENTS.md:L96-L100
Useful? React with 👍 / 👎.
| let registration = azureRequestHeaders.get(headers); | ||
| if (!registration) { |
There was a problem hiding this comment.
Isolate cached header snapshots per request
When two body-bearing requests overlap while reusing the same mutable header record, the second call reuses this registration and its already-cached carrier instead of capturing the record's current value. For example, if request A snapshots an api-key for tenant A and pauses in authentication, then the caller updates that record for tenant B and starts request B—even through a different AzureOpenAI client—request B still dispatches tenant A's credential. The remaining case after preserving shared options identity is mutation between concurrent invocations; each request needs its own snapshot rather than a module-global snapshot keyed only by the caller object.
AGENTS.md reference: AGENTS.md:L114-L117
Useful? React with 👍 / 👎.
Summary
TypeErrorthat cannot expose a credential through error messages, stacks, or logging.Regression-first proof
The new real-public-client regression produced 214 failing / 17 passing cases before the production change; all 231 cases pass afterward. It covers static and rotating credentials across generic requests, model listings, and chat completions, all forbidden C0 bytes, DEL, CRLF, astral Unicode, unpaired surrogates, default/custom logging, absent causes, and zero outbound requests.
Verification
publintonly reports its pre-existing vendor warning.