fix: allow api_key="" to bypass credential validation for local servers - #3225
fix: allow api_key="" to bypass credential validation for local servers#3225tbille wants to merge 3 commits into
Conversation
![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: 168a416518
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
089169b to
b0cb6cf
Compare
b0cb6cf to
dc74198
Compare
|
can we get an update on this issue? |
397125f to
f96258a
Compare
f96258a to
e1e32dd
Compare
e1e32dd to
5d0d0ad
Compare
In v2.34.0, the credential validation changed from an identity check (api_key is None) to a truthiness check (not self.api_key), which caused api_key="" to be rejected as missing credentials. This broke OpenAI-compatible local servers (llama.cpp, llamafile, LM Studio, vLLM) that don't require authentication. Track whether api_key was explicitly provided by the caller and skip the credential error when it was, even if the value is an empty string. Fixes openai#3224
Move _api_key_explicitly_set check before the OPENAI_API_KEY env var lookup so that only caller-provided api_key="" bypasses validation. OPENAI_API_KEY="" in the environment (likely misconfiguration) still raises the Missing credentials error. Add tests for the OPENAI_API_KEY="" env var scenario.
5d0d0ad to
c8c6694
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8c66949e1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| provider_runtime is None | ||
| and _enforce_credentials | ||
| and not self.api_key | ||
| and not _api_key_explicitly_set |
There was a problem hiding this comment.
When a caller explicitly passes api_key="" for an auth-less local server, this constructor check now allows the client to be created, but the first normal bearer-auth request still fails: _bearer_auth/auth_headers return no Authorization header for an empty key, and _build_headers() then calls _validate_headers(), which raises because no auth header was resolved. The same pattern exists in AsyncOpenAI, so the new path only passes construction tests and still cannot make typical SDK calls unless users manually omit or provide an Authorization header.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — confirmed the empty api_key was still rejected at request time. Fixed in 3214527: added _api_key_explicitly_empty tracking (limited to a literal explicitly-passed api_key="", not key-providers or workload identity) and skip the auth-header validation for that case in _validate_headers, unless the request specifically requires admin_api_key_auth (which an empty api_key still can't satisfy). Had to thread security into _validate_headers (and update the AzureOpenAI overrides) to make that distinction precisely. Added regression tests that build actual requests (not just construct the client) for both the bearer_auth-succeeds and admin_api_key_auth-still-raises cases.
Client construction with api_key="" was fixed to not raise "Missing credentials", but _validate_headers still rejected the first actual request for bearer_auth endpoints since no Authorization header could be built from an empty key. This left the local-server use case (llama.cpp, LM Studio, vLLM, etc.) broken end-to-end. Track whether the caller passed a literal empty string (as opposed to a key provider, workload identity, or unset value) and skip the 'Could not resolve authentication method' error for such clients, except when the request specifically requires admin credentials that an empty api_key cannot satisfy. Thread the request's security requirements into _validate_headers to make that distinction, updating the AzureOpenAI overrides to match the new signature. Addresses review feedback from PR openai#3225.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3214527305
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # don't fail requests just because no `Authorization` header could be built — | ||
| # unless the request specifically requires admin credentials, which an empty | ||
| # `api_key` cannot satisfy. | ||
| if self._api_key_explicitly_empty and not (security or {}).get("admin_api_key_auth", False): |
There was a problem hiding this comment.
When api_key="" is used with the public raw helpers (client.get("/foo", ...), post, etc.) and no options["security"] override is supplied, FinalRequestOptions still has its default admin_api_key_auth=True. This check therefore treats the request as admin-only and raises before it reaches an auth-less local/custom endpoint, even though generated endpoints pass security={"bearer_auth": True}. The fresh evidence beyond the earlier auth-header finding is that the raw helper path constructs options without a security override, so the default admin flag still blocks the new empty-key mode.
Useful? React with 👍 / 👎.
| # This is needed so that requests don't fail header validation below when the | ||
| # caller intentionally disabled authentication (e.g. for local, auth-less | ||
| # OpenAI-compatible servers). | ||
| self._api_key_explicitly_empty = api_key == "" |
There was a problem hiding this comment.
This new explicit-empty state is only captured when the constructor receives api_key="", but other supported configuration paths do not preserve that intent: copy()/with_options() still uses api_key or self._api_key_provider or self.api_key, so client.with_options(api_key="", base_url=local_url) inherits and sends the previous non-empty key instead of disabling auth, and an already-loaded module client whose openai.api_key is later set to "" never updates this flag and still fails bearer-auth request validation. Please thread the explicit-empty state through these reconfiguration paths so local auth-less servers work consistently.
Useful? React with 👍 / 👎.
1 participant
Summary
Fixes #3224
_client.pychanged from an identity check (api_key is None) to a truthiness check (not self.api_key), which causesapi_key=""to be rejected as "Missing credentials"api_key=""api_keywas explicitly provided by the caller and skips the credential error when it was, even if the value is an empty stringChanges
src/openai/_client.py: Added_api_key_explicitly_setlocal variable in bothOpenAI.__init__()andAsyncOpenAI.__init__()that tracks whether the caller explicitly passed anapi_keyargument (vs it being resolved from the environment or defaulting toNone). The credential validation check now also considers this flag, soapi_key=""no longer triggers the error whileapi_key=Nonewith no env var still does.tests/test_client.py: Added test cases for both sync and async clients verifying thatapi_key=""does not raiseOpenAIError, restoring the pre-v2.34.0 behavior.