From c82f1fcbee3336919964bba98fc9c61675340c6d Mon Sep 17 00:00:00 2001 From: kamilbenkirane Date: Wed, 25 Feb 2026 13:23:54 +0100 Subject: [PATCH 1/3] fix: empty/whitespace credentials should raise MissingCredentialsError Closes #184 --- src/celeste/credentials.py | 5 +++-- tests/unit_tests/test_credentials.py | 17 +++++++++-------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/celeste/credentials.py b/src/celeste/credentials.py index 2feb118f..336eb872 100644 --- a/src/celeste/credentials.py +++ b/src/celeste/credentials.py @@ -116,7 +116,7 @@ def get_credentials( field_name = secret_name.lower() credential: SecretStr | None = getattr(self, field_name, None) - if credential is None: + if credential is None or not credential.get_secret_value().strip(): raise MissingCredentialsError(provider=provider) return credential @@ -142,7 +142,8 @@ def has_credential(self, provider: Provider) -> bool: secret_name, _, _ = registered field_name = secret_name.lower() - return getattr(self, field_name, None) is not None + credential = getattr(self, field_name, None) + return credential is not None and bool(credential.get_secret_value().strip()) def get_auth( self, diff --git a/tests/unit_tests/test_credentials.py b/tests/unit_tests/test_credentials.py index 5a22df4c..6b6b55b3 100644 --- a/tests/unit_tests/test_credentials.py +++ b/tests/unit_tests/test_credentials.py @@ -362,23 +362,24 @@ class TestEdgeCases: """Test edge cases and error conditions.""" @pytest.mark.parametrize("value", ["", " ", "\t\n"]) - def test_whitespace_credentials( + def test_whitespace_credentials_are_treated_as_missing( self, monkeypatch: pytest.MonkeyPatch, value: str ) -> None: - """Test handling of whitespace-only credentials. + """Test that empty/whitespace-only credentials are treated as missing. - Note: Currently treats whitespace as 'present' credentials. - This is a deliberate choice - empty/whitespace values are still - considered as having credentials set (may be used for optional APIs). + When an env var is set to empty string or whitespace (e.g. OPENAI_API_KEY=""), + Pydantic BaseSettings produces SecretStr("") instead of None. The SDK must + detect this and raise MissingCredentialsError rather than sending invalid + Authorization headers. """ # Arrange monkeypatch.setenv("OPENAI_API_KEY", value) creds = Credentials() # type: ignore[call-arg] # Act & Assert - assert creds.has_credential(Provider.OPENAI) is True - credential = creds.get_credentials(Provider.OPENAI) - assert credential.get_secret_value() == value + assert creds.has_credential(Provider.OPENAI) is False + with pytest.raises(MissingCredentialsError): + creds.get_credentials(Provider.OPENAI) def test_special_characters_in_credentials( self, monkeypatch: pytest.MonkeyPatch From 2b6562b4f6cf9454517778646b54b20410144c75 Mon Sep 17 00:00:00 2001 From: kamilbenkirane Date: Wed, 25 Feb 2026 13:52:31 +0100 Subject: [PATCH 2/3] fix: extend whitespace credential check to override_key path Also updates get_credentials and has_credential docstrings to reflect the new empty/whitespace-only rejection behavior. --- src/celeste/credentials.py | 16 +++++++++++----- tests/unit_tests/test_credentials.py | 8 ++++++++ 2 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/celeste/credentials.py b/src/celeste/credentials.py index 336eb872..55022b25 100644 --- a/src/celeste/credentials.py +++ b/src/celeste/credentials.py @@ -96,12 +96,18 @@ def get_credentials( SecretStr containing the API key for the provider. Raises: - MissingCredentialsError: If provider requires credentials but none are configured. + MissingCredentialsError: If provider requires credentials but none are configured, + or the configured value is empty/whitespace-only. """ if override_key: - if isinstance(override_key, str): - return SecretStr(override_key) - return override_key + raw = ( + override_key + if isinstance(override_key, str) + else override_key.get_secret_value() + ) + if not raw.strip(): + raise MissingCredentialsError(provider=provider) + return SecretStr(raw) if isinstance(override_key, str) else override_key registered = _auth_registry.get(provider) if registered is None: @@ -130,7 +136,7 @@ def list_available_providers(self) -> list[Provider]: ] def has_credential(self, provider: Provider) -> bool: - """Check if a specific provider has credentials configured.""" + """Check if a specific provider has non-empty credentials configured.""" registered = _auth_registry.get(provider) if registered is None: raise UnsupportedProviderError(provider=provider) diff --git a/tests/unit_tests/test_credentials.py b/tests/unit_tests/test_credentials.py index 6b6b55b3..46c24ea6 100644 --- a/tests/unit_tests/test_credentials.py +++ b/tests/unit_tests/test_credentials.py @@ -381,6 +381,14 @@ def test_whitespace_credentials_are_treated_as_missing( with pytest.raises(MissingCredentialsError): creds.get_credentials(Provider.OPENAI) + @pytest.mark.parametrize("value", ["", " ", "\t\n"]) + def test_whitespace_override_key_treated_as_missing(self, value: str) -> None: + """Test that empty/whitespace override_key raises MissingCredentialsError.""" + creds = Credentials() # type: ignore[call-arg] + + with pytest.raises(MissingCredentialsError): + creds.get_credentials(Provider.OPENAI, override_key=value) + def test_special_characters_in_credentials( self, monkeypatch: pytest.MonkeyPatch ) -> None: From a87572bd65e69cf04abd7c7d3afed9e993f75415 Mon Sep 17 00:00:00 2001 From: kamilbenkirane Date: Wed, 25 Feb 2026 14:04:39 +0100 Subject: [PATCH 3/3] fix: use `is not None` guard for override_key to catch empty strings `if override_key:` was falsy for "" and SecretStr(""), silently falling through to env-var lookup instead of raising MissingCredentialsError. Also adds SecretStr coverage and a fallthrough regression test. --- src/celeste/credentials.py | 2 +- tests/unit_tests/test_credentials.py | 20 ++++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/celeste/credentials.py b/src/celeste/credentials.py index 55022b25..4e6cd29c 100644 --- a/src/celeste/credentials.py +++ b/src/celeste/credentials.py @@ -99,7 +99,7 @@ def get_credentials( MissingCredentialsError: If provider requires credentials but none are configured, or the configured value is empty/whitespace-only. """ - if override_key: + if override_key is not None: raw = ( override_key if isinstance(override_key, str) diff --git a/tests/unit_tests/test_credentials.py b/tests/unit_tests/test_credentials.py index 46c24ea6..91decfce 100644 --- a/tests/unit_tests/test_credentials.py +++ b/tests/unit_tests/test_credentials.py @@ -381,14 +381,30 @@ def test_whitespace_credentials_are_treated_as_missing( with pytest.raises(MissingCredentialsError): creds.get_credentials(Provider.OPENAI) - @pytest.mark.parametrize("value", ["", " ", "\t\n"]) - def test_whitespace_override_key_treated_as_missing(self, value: str) -> None: + @pytest.mark.parametrize( + "value", + ["", " ", "\t\n", SecretStr(""), SecretStr(" "), SecretStr("\t\n")], + ) + def test_whitespace_override_key_treated_as_missing( + self, value: str | SecretStr + ) -> None: """Test that empty/whitespace override_key raises MissingCredentialsError.""" creds = Credentials() # type: ignore[call-arg] with pytest.raises(MissingCredentialsError): creds.get_credentials(Provider.OPENAI, override_key=value) + @pytest.mark.parametrize("value", ["", " ", SecretStr(""), SecretStr(" ")]) + def test_whitespace_override_key_not_fallthrough( + self, monkeypatch: pytest.MonkeyPatch, value: str | SecretStr + ) -> None: + """Test that empty override_key raises even when a valid env var exists.""" + monkeypatch.setenv("OPENAI_API_KEY", "valid-env-key") + creds = Credentials() # type: ignore[call-arg] + + with pytest.raises(MissingCredentialsError): + creds.get_credentials(Provider.OPENAI, override_key=value) + def test_special_characters_in_credentials( self, monkeypatch: pytest.MonkeyPatch ) -> None: