diff --git a/src/celeste/credentials.py b/src/celeste/credentials.py index 2feb118f..4e6cd29c 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 + if override_key is not None: + 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: @@ -116,7 +122,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 @@ -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) @@ -142,7 +148,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..91decfce 100644 --- a/tests/unit_tests/test_credentials.py +++ b/tests/unit_tests/test_credentials.py @@ -362,23 +362,48 @@ 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) + + @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