-
Notifications
You must be signed in to change notification settings - Fork 5.1k
fix: allow api_key="" to bypass credential validation for local servers #3225
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -197,6 +197,14 @@ def __init__( | |
|
|
||
| self.workload_identity = workload_identity if provider_runtime is None else None | ||
|
|
||
| _api_key_explicitly_set = api_key is not None | ||
| # Tracks whether the caller explicitly passed a literal `api_key=""`, as opposed | ||
| # to it defaulting to an empty string because no credentials were configured, or | ||
| # a key provider/workload identity being used (which resolve the real key later). | ||
| # 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 == "" | ||
| if provider_runtime is not None: | ||
| self.api_key = "" | ||
| self._api_key_provider = None | ||
|
|
@@ -224,6 +232,7 @@ def __init__( | |
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a caller explicitly passes Useful? React with 👍 / 👎.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch — confirmed the empty api_key was still rejected at request time. Fixed in 3214527: added |
||
| and self._api_key_provider is None | ||
| and workload_identity is None | ||
| and self.admin_api_key is None | ||
|
|
@@ -544,13 +553,23 @@ def default_headers(self) -> dict[str, str | Omit]: | |
| } | ||
|
|
||
| @override | ||
| def _validate_headers(self, headers: Headers, custom_headers: Headers) -> None: | ||
| def _validate_headers( | ||
| self, headers: Headers, custom_headers: Headers, security: SecurityOptions | None = None | ||
| ) -> None: | ||
| if self._provider_runtime is not None: | ||
| return | ||
|
|
||
| if _has_header(headers, "Authorization") or _has_omitted_header(custom_headers, "Authorization"): | ||
| return | ||
|
|
||
| # An explicitly-passed `api_key=""` means the caller intentionally disabled | ||
| # authentication (e.g. for a local, auth-less OpenAI-compatible server), so | ||
| # 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| return | ||
|
|
||
| raise TypeError( | ||
| '"Could not resolve authentication method. Expected either api_key or admin_api_key to be set. Or for one of the `Authorization` or `Authorization` headers to be explicitly omitted"' | ||
| ) | ||
|
|
@@ -803,6 +822,14 @@ def __init__( | |
|
|
||
| self.workload_identity = workload_identity if provider_runtime is None else None | ||
|
|
||
| _api_key_explicitly_set = api_key is not None | ||
| # Tracks whether the caller explicitly passed a literal `api_key=""`, as opposed | ||
| # to it defaulting to an empty string because no credentials were configured, or | ||
| # a key provider/workload identity being used (which resolve the real key later). | ||
| # 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 == "" | ||
| if provider_runtime is not None: | ||
| self.api_key = "" | ||
| self._api_key_provider = None | ||
|
|
@@ -830,6 +857,7 @@ def __init__( | |
| provider_runtime is None | ||
| and _enforce_credentials | ||
| and not self.api_key | ||
| and not _api_key_explicitly_set | ||
| and self._api_key_provider is None | ||
| and workload_identity is None | ||
| and self.admin_api_key is None | ||
|
|
@@ -1153,13 +1181,23 @@ def default_headers(self) -> dict[str, str | Omit]: | |
| } | ||
|
|
||
| @override | ||
| def _validate_headers(self, headers: Headers, custom_headers: Headers) -> None: | ||
| def _validate_headers( | ||
| self, headers: Headers, custom_headers: Headers, security: SecurityOptions | None = None | ||
| ) -> None: | ||
| if self._provider_runtime is not None: | ||
| return | ||
|
|
||
| if _has_header(headers, "Authorization") or _has_omitted_header(custom_headers, "Authorization"): | ||
| return | ||
|
|
||
| # An explicitly-passed `api_key=""` means the caller intentionally disabled | ||
| # authentication (e.g. for a local, auth-less OpenAI-compatible server), so | ||
| # 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): | ||
| return | ||
|
|
||
| raise TypeError( | ||
| '"Could not resolve authentication method. Expected either api_key or admin_api_key to be set. Or for one of the `Authorization` or `Authorization` headers to be explicitly omitted"' | ||
| ) | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 usesapi_key or self._api_key_provider or self.api_key, soclient.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 whoseopenai.api_keyis 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 👍 / 👎.