Skip to content

fix(security): redact invalid Azure authentication credentials - #2421

Open
HAYDEN-OAI wants to merge 16 commits into
mainfrom
codex/azure-credential-header-privacy-20260819-2019
Open

fix(security): redact invalid Azure authentication credentials#2421
HAYDEN-OAI wants to merge 16 commits into
mainfrom
codex/azure-credential-header-privacy-20260819-2019

Conversation

@HAYDEN-OAI

Copy link
Copy Markdown
Contributor

Summary

  • Validate resolved Azure static API keys and rotating Microsoft Entra bearer tokens before either credential is attached to an HTTP request.
  • Reject forbidden HTTP control characters, DEL, and non-byte Unicode using a fixed, cause-free TypeError that cannot expose a credential through error messages, stacks, or logging.
  • Preserve horizontal tabs, spaces, and valid HTTP obs-text; authentication opt-outs, provider errors, caller-header diagnostics, request redirects, and credential refresh behavior.
  • Keep the fix exclusively in handwritten Azure authentication code; generated core client files remain untouched.

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

  • 348 focused Azure tests passing.
  • 4,074 handwritten tests across 123 files passing.
  • 559 generated tests across 82 files passing against an isolated random-port mock.
  • Repository formatting/lint, strict TypeScript, and full CommonJS/ESM package build.
  • Published-source TypeScript 4.9 and current TypeScript; publint only reports its pre-existing vendor warning.
  • Packed-package Node 22 smoke checks, including 136 rejected malformed credentials and 16 valid compatibility cases across CommonJS/ESM and both Azure authentication modes.

@HAYDEN-OAI
HAYDEN-OAI requested a review from a team as a code owner August 19, 2026 20:32
@openai-sdks

openai-sdks Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

OkTest Summary

237/237 SDK tests passed in 10.418s for Node SDK PR #2421.

Test results — 42 files
Test Result Time
tests/chat-completions-complex-body.test.ts ✅ Passed 164ms
tests/chat-completions-create.test.ts ✅ Passed 284ms
tests/chat-completions-stream.test.ts ✅ Passed 149ms
tests/files-content-binary.test.ts ✅ Passed 146ms
tests/files-create-multipart.test.ts ✅ Passed 149ms
tests/files-list-pagination.test.ts ✅ Passed 142ms
tests/initialize-config.test.ts ✅ Passed 145ms
tests/instance-isolation.test.ts ✅ Passed 129ms
tests/models-list.test.ts ✅ Passed 135ms
tests/responses-background-lifecycle.test.ts ✅ Passed 207ms
tests/responses-body-method-errors.test.ts ✅ Passed 292ms
tests/responses-cancel-timeout.test.ts ✅ Passed 212ms
tests/responses-cancel.test.ts ✅ Passed 263ms
tests/responses-compact-retries.test.ts ✅ Passed 233ms
tests/responses-compact.test.ts ✅ Passed 241ms
tests/responses-create-advanced-stream.test.ts ✅ Passed 106ms
tests/responses-create-advanced.test.ts ✅ Passed 197ms
tests/responses-create-disconnect.test.ts ✅ Passed 1.129s
tests/responses-create-errors.test.ts ✅ Passed 200ms
tests/responses-create-malformed-api-responses.test.ts ✅ Passed 142ms
tests/responses-create-retries.test.ts ✅ Passed 251ms
tests/responses-create-stream-failures.test.ts ✅ Passed 229ms
tests/responses-create-stream-timeout.test.ts ✅ Passed 2.154s
tests/responses-create-stream-wire.test.ts ✅ Passed 2.591s
tests/responses-create-stream.test.ts ✅ Passed 81ms
tests/responses-create-terminal-states.test.ts ✅ Passed 228ms
tests/responses-create-timeout.test.ts ✅ Passed 228ms
tests/responses-create.test.ts ✅ Passed 187ms
tests/responses-delete.test.ts ✅ Passed 194ms
tests/responses-input-items-errors.test.ts ✅ Passed 179ms
tests/responses-input-items-list.test.ts ✅ Passed 185ms
tests/responses-input-items-options.test.ts ✅ Passed 108ms
tests/responses-input-tokens-count-timeout.test.ts ✅ Passed 232ms
tests/responses-input-tokens-count.test.ts ✅ Passed 278ms
tests/responses-malformed-inputs.test.ts ✅ Passed 2.151s
tests/responses-not-found-errors.test.ts ✅ Passed 231ms
tests/responses-parse.test.ts ✅ Passed 210ms
tests/responses-retrieve-retries.test.ts ✅ Passed 319ms
tests/responses-retrieve.test.ts ✅ Passed 232ms
tests/responses-stored-method-errors.test.ts ✅ Passed 529ms
tests/retry-behavior.test.ts ✅ Passed 3.208s
tests/sdk-error-shape.test.ts ✅ Passed 249ms

View OkTest run #32325226542

SDK merge (5f8ffd544faf) · head (59af3c0ce2f0) · base (dcc705c118fe) · OkTest (2b1bdfd25e98)

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/azure.ts Outdated
Comment thread src/azure.ts Outdated

@jbeckwith-oai jbeckwith-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread tests/lib/azure-deployment-path-safety.test.ts Fixed

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/headers.ts Outdated
Comment thread src/azure.ts Outdated
Comment thread src/internal/headers.ts Outdated
Comment thread src/azure.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/azure.ts Outdated
Comment thread src/internal/headers.ts
Comment thread src/azure.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/headers.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/headers.ts
@github-actions

github-actions Bot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

Castiron custom code

⚠️ Report unavailable for 59af3c0ce2f0.

The report setup or validation failed. Inspect the workflow run.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/headers.ts Outdated

@jbeckwith-oai jbeckwith-oai left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread src/internal/headers.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/azure.ts Outdated
Comment thread src/internal/headers.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/headers.ts Outdated
Comment thread src/internal/headers.ts
Comment thread src/internal/headers.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/azure.ts Outdated
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Aug 20, 2026

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-08-20T02:36:34.788690Z 59af3c0 New commits
🔒 Security Review 🔄 Running since 2026-08-20T02:36:34.899795Z 59af3c0 New commits
ℹ️ 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" or "@codex security review".

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 chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/headers.ts
Comment thread src/internal/azure.ts Outdated
Comment thread src/internal/headers.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/internal/azure.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/azure.ts Outdated
Comment thread src/azure.ts

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/azure.ts Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/azure.ts
);

function hasIntrinsicHeadersIdentity(headers: RequestInit['headers']): headers is Headers {
if (!(headers instanceof Headers) || Object.getPrototypeOf(headers) !== intrinsicHeadersPrototype) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

Comment thread src/azure.ts
Comment on lines +177 to +179
const { body, headers } = options;
const preprocessesHeaders = body === undefined ? 'body' in options : Boolean(body);
const protection = preprocessesHeaders ? protectAzureRequestHeaders(headers) : undefined;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Comment thread src/internal/headers.ts
Comment on lines +368 to +369
let registration = azureRequestHeaders.get(headers);
if (!registration) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge 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 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants