Skip to content

fix: strip api key on cross-origin redirects#3518

Open
nightcityblade wants to merge 1 commit into
openai:mainfrom
nightcityblade:fix/issue-3516
Open

fix: strip api key on cross-origin redirects#3518
nightcityblade wants to merge 1 commit into
openai:mainfrom
nightcityblade:fix/issue-3516

Conversation

@nightcityblade

Copy link
Copy Markdown
  • I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

Fixes #3516.

Strip the Azure api-key credential when the SDK's default HTTP clients follow a redirect to a different origin. This keeps the existing same-origin redirect behavior and covers the synchronous, asynchronous, and aiohttp-backed default clients.

The origin comparison normalizes default HTTP/HTTPS ports, and a regression test exercises both the sync and async client implementations.

Additional context & links

Validation completed:

  • .venv/bin/pytest -q tests/test_client.py -k 'cross_origin_redirect_strips_api_key or follow_redirects' — 6 passed.
  • .venv/bin/ruff format --check src/openai/_base_client.py tests/test_client.py — passed.
  • .venv/bin/ruff check src/openai/_base_client.py tests/test_client.py — passed.
  • .venv/bin/pyright --pythonpath .venv/bin/python src/openai/_base_client.py — passed with 0 errors.
  • .venv/bin/mypy src/openai/_base_client.py — passed with 0 issues.

@rkfshakti rkfshakti 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.

Important security fix. The origin() comparison correctly handles the default port logic (443 for https, 80 for http), which is a common edge case that naive URL comparison gets wrong.

The approach of overriding _build_redirect_request in both DefaultHttpxClient and DefaultAsyncHttpxClient is the right hook point — it intercepts redirects at the transport layer before any auth headers are applied to the new request.

One observation: the origin() helper strips the API key from headers but not from the URL query string or body. If the API key could ever appear in those locations (e.g., ?api_key=...), this wouldn't catch it. But looking at the codebase, the key is only passed via the api-key header, so this is correct for the current usage.

LGTM.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Azure api-key header is forwarded across cross-origin redirects (httpx strips Authorization but not custom auth headers)

2 participants