Skip to content

Commit 3921847

Browse files
authored
fix: Harden secret handling and passthrough fallbacks (#5799)
# What does this PR do? This PR hardens OAuth2 introspection secret handling by using `SecretStr` in config and unwrapping secret values only when sending introspection requests. It tightens messages passthrough fallbacks by catching only expected lookup errors and adding debug logs instead of silently swallowing failures. It also improves vector-store provider lookup error logging, removes a duplicate `ResponseNotFoundError` export, and drops an outdated TODO comment. ## Test Plan - `uv run pytest tests/unit/server/test_auth_oauth2_introspection.py tests/unit/providers/inline/messages/test_impl.py tests/unit/telemetry/test_vector_io_metrics.py -q --maxfail=1` - Result: `36 passed in 0.32s` Signed-off-by: Sébastien Han <seb@redhat.com>
1 parent c3fe284 commit 3921847

6 files changed

Lines changed: 10 additions & 14 deletions

File tree

src/ogx/core/datatypes.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
from typing import Annotated, Any, Literal, Self
1111
from urllib.parse import urlparse
1212

13-
from pydantic import BaseModel, Field, field_validator, model_validator
13+
from pydantic import BaseModel, Field, SecretStr, field_validator, model_validator
1414

1515
from ogx.core.access_control.datatypes import AccessRule, RouteAccessRule
1616
from ogx.core.storage.datatypes import (
@@ -193,7 +193,7 @@ class OAuth2IntrospectionConfig(BaseModel):
193193

194194
url: str
195195
client_id: str
196-
client_secret: str
196+
client_secret: SecretStr
197197
send_secret_in_body: bool = False
198198

199199

src/ogx/core/resolver.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ def additional_protocols_map() -> dict[Api, Any]:
132132
}
133133

134134

135-
# TODO: make all this naming far less atrocious. Provider. ProviderSpec. ProviderWithSpec. WTF!
136135
class ProviderWithSpec(Provider):
137136
"""A Provider paired with its resolved ProviderSpec for instantiation."""
138137

src/ogx/core/routers/vector_io.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,7 @@ def _get_provider_id(self, vector_store_id: str) -> str:
103103
return "unknown"
104104
return obj.provider_id
105105
except Exception:
106-
logger.warning(
107-
"Could not resolve provider for vector store", vector_store_id=vector_store_id, exc_info=True
108-
)
106+
logger.exception("Could not resolve provider for vector store", vector_store_id=vector_store_id)
109107
return "unknown"
110108

111109
async def _rewrite_query_for_search(self, query: str) -> str:

src/ogx/core/server/auth_providers.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -246,12 +246,12 @@ async def introspect_token(self, token: str, scope: Scope | None = None) -> User
246246

247247
if self.config.introspection.send_secret_in_body:
248248
form["client_id"] = self.config.introspection.client_id
249-
form["client_secret"] = self.config.introspection.client_secret
249+
form["client_secret"] = self.config.introspection.client_secret.get_secret_value()
250250
else:
251251
# httpx auth parameter expects tuple[str | bytes, str | bytes]
252252
post_kwargs["auth"] = (
253253
self.config.introspection.client_id,
254-
self.config.introspection.client_secret,
254+
self.config.introspection.client_secret.get_secret_value(),
255255
)
256256

257257
try:

src/ogx/providers/inline/messages/impl.py

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ async def _get_passthrough_url(self, model: str) -> str | None:
155155
base_url = base_url[:-3]
156156
logger.info("Using native /v1/messages passthrough", model=model, base_url=base_url)
157157
return base_url
158-
except Exception:
158+
except (KeyError, ValueError, AttributeError):
159159
logger.debug("Failed to resolve passthrough, falling back to translation", model=model)
160160

161161
return None
@@ -175,8 +175,8 @@ async def _passthrough_request(
175175
obj = await router.routing_table.get_object_by_identifier("model", request.model)
176176
if obj:
177177
provider_model = obj.provider_resource_id
178-
except Exception:
179-
pass
178+
except (KeyError, ValueError, AttributeError):
179+
logger.debug("Failed to resolve provider model name, using original", model=request.model)
180180

181181
body = request.model_dump(exclude_none=True)
182182
body["model"] = provider_model
@@ -268,8 +268,8 @@ async def _passthrough_count_tokens(
268268
obj = await router.routing_table.get_object_by_identifier("model", request.model)
269269
if obj:
270270
provider_model = obj.provider_resource_id
271-
except Exception:
272-
pass
271+
except (KeyError, ValueError, AttributeError):
272+
logger.debug("Failed to resolve provider model name, using original", model=request.model)
273273

274274
body = request.model_dump(exclude_none=True)
275275
body["model"] = provider_model

src/ogx_api/__init__.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -889,7 +889,6 @@
889889
"ResponseFormatType",
890890
"ResponseItemInclude",
891891
"ResponseTruncation",
892-
"ResponseNotFoundError",
893892
"ResponseStreamOptions",
894893
"RetrieveFileContentRequest",
895894
"RetrieveFileRequest",

0 commit comments

Comments
 (0)