Skip to content

Commit 3ef3f9d

Browse files
fix: empty/whitespace credentials should raise MissingCredentialsError (#190)
* fix: empty/whitespace credentials should raise MissingCredentialsError Closes #184 * 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. * 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.
1 parent 0a94f5a commit 3ef3f9d

2 files changed

Lines changed: 48 additions & 16 deletions

File tree

src/celeste/credentials.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,12 +96,18 @@ def get_credentials(
9696
SecretStr containing the API key for the provider.
9797
9898
Raises:
99-
MissingCredentialsError: If provider requires credentials but none are configured.
99+
MissingCredentialsError: If provider requires credentials but none are configured,
100+
or the configured value is empty/whitespace-only.
100101
"""
101-
if override_key:
102-
if isinstance(override_key, str):
103-
return SecretStr(override_key)
104-
return override_key
102+
if override_key is not None:
103+
raw = (
104+
override_key
105+
if isinstance(override_key, str)
106+
else override_key.get_secret_value()
107+
)
108+
if not raw.strip():
109+
raise MissingCredentialsError(provider=provider)
110+
return SecretStr(raw) if isinstance(override_key, str) else override_key
105111

106112
registered = _auth_registry.get(provider)
107113
if registered is None:
@@ -116,7 +122,7 @@ def get_credentials(
116122
field_name = secret_name.lower()
117123

118124
credential: SecretStr | None = getattr(self, field_name, None)
119-
if credential is None:
125+
if credential is None or not credential.get_secret_value().strip():
120126
raise MissingCredentialsError(provider=provider)
121127

122128
return credential
@@ -130,7 +136,7 @@ def list_available_providers(self) -> list[Provider]:
130136
]
131137

132138
def has_credential(self, provider: Provider) -> bool:
133-
"""Check if a specific provider has credentials configured."""
139+
"""Check if a specific provider has non-empty credentials configured."""
134140
registered = _auth_registry.get(provider)
135141
if registered is None:
136142
raise UnsupportedProviderError(provider=provider)
@@ -142,7 +148,8 @@ def has_credential(self, provider: Provider) -> bool:
142148
secret_name, _, _ = registered
143149
field_name = secret_name.lower()
144150

145-
return getattr(self, field_name, None) is not None
151+
credential = getattr(self, field_name, None)
152+
return credential is not None and bool(credential.get_secret_value().strip())
146153

147154
def get_auth(
148155
self,

tests/unit_tests/test_credentials.py

Lines changed: 33 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -362,23 +362,48 @@ class TestEdgeCases:
362362
"""Test edge cases and error conditions."""
363363

364364
@pytest.mark.parametrize("value", ["", " ", "\t\n"])
365-
def test_whitespace_credentials(
365+
def test_whitespace_credentials_are_treated_as_missing(
366366
self, monkeypatch: pytest.MonkeyPatch, value: str
367367
) -> None:
368-
"""Test handling of whitespace-only credentials.
368+
"""Test that empty/whitespace-only credentials are treated as missing.
369369
370-
Note: Currently treats whitespace as 'present' credentials.
371-
This is a deliberate choice - empty/whitespace values are still
372-
considered as having credentials set (may be used for optional APIs).
370+
When an env var is set to empty string or whitespace (e.g. OPENAI_API_KEY=""),
371+
Pydantic BaseSettings produces SecretStr("") instead of None. The SDK must
372+
detect this and raise MissingCredentialsError rather than sending invalid
373+
Authorization headers.
373374
"""
374375
# Arrange
375376
monkeypatch.setenv("OPENAI_API_KEY", value)
376377
creds = Credentials() # type: ignore[call-arg]
377378

378379
# Act & Assert
379-
assert creds.has_credential(Provider.OPENAI) is True
380-
credential = creds.get_credentials(Provider.OPENAI)
381-
assert credential.get_secret_value() == value
380+
assert creds.has_credential(Provider.OPENAI) is False
381+
with pytest.raises(MissingCredentialsError):
382+
creds.get_credentials(Provider.OPENAI)
383+
384+
@pytest.mark.parametrize(
385+
"value",
386+
["", " ", "\t\n", SecretStr(""), SecretStr(" "), SecretStr("\t\n")],
387+
)
388+
def test_whitespace_override_key_treated_as_missing(
389+
self, value: str | SecretStr
390+
) -> None:
391+
"""Test that empty/whitespace override_key raises MissingCredentialsError."""
392+
creds = Credentials() # type: ignore[call-arg]
393+
394+
with pytest.raises(MissingCredentialsError):
395+
creds.get_credentials(Provider.OPENAI, override_key=value)
396+
397+
@pytest.mark.parametrize("value", ["", " ", SecretStr(""), SecretStr(" ")])
398+
def test_whitespace_override_key_not_fallthrough(
399+
self, monkeypatch: pytest.MonkeyPatch, value: str | SecretStr
400+
) -> None:
401+
"""Test that empty override_key raises even when a valid env var exists."""
402+
monkeypatch.setenv("OPENAI_API_KEY", "valid-env-key")
403+
creds = Credentials() # type: ignore[call-arg]
404+
405+
with pytest.raises(MissingCredentialsError):
406+
creds.get_credentials(Provider.OPENAI, override_key=value)
382407

383408
def test_special_characters_in_credentials(
384409
self, monkeypatch: pytest.MonkeyPatch

0 commit comments

Comments
 (0)