feat(core): normalize inference provider-data key by provider id - #6468
Conversation
Per-request credentials for inference providers are passed via the
X-OGX-Provider-Data header, keyed by each provider's internal
provider_data_api_key_field. That field is shaped by the backend type
(e.g. vllm_api_token for a remote::vllm provider), so the header leaked
the provider type to the client. A user had to know that a provider was
backed by vLLM in order to send the correct per-request credential key,
which defeats the point of addressing a provider purely by its name. It
also made it impossible to run two providers of the same type and give
each a distinct credential, since both shared the same type-shaped key.
Resolve the leak by normalizing the key in the inference router. The
router maps the client-facing {provider_id}_api_key onto the provider's
internal provider_data_api_key_field on the request provider-data
context before dispatching. Clients authenticate a named provider
without knowing its backend type, and same-type providers can each be
keyed by their own provider id (e.g. two remote::vllm providers using
providerA_api_key and providerB_api_key).
The mapping lives in the router rather than in the provider
implementations so each provider's per-type provider-data validator is
left untouched: it still observes the internal key it already knows.
The legacy type-shaped key keeps working unchanged for backward
compatibility, and when both are present the provider-id key takes
precedence. The missing-key error in the OpenAI mixin now references
{provider_id}_api_key instead of the backend type, closing the
remaining leak.
Scope: the inference router, covering the OpenAI-mixin inference
providers. Non-OpenAI remote providers and other router surfaces are
follow-up work.
Adds unit tests for the normalization helper, the router call-site, and
the error-message branches, plus integration tests exercising the
provider-id key against a vllm provider, and documents the convention
in the "Using OGX as a Library" guide.
Signed-off-by: Matthew Farrellee <matt@cs.wisc.edu>
|
@franciscojavierarceo fyi, this should address a core Chat Playground UI use case |
VANDRANKI
left a comment
VANDRANKI
left a comment
There was a problem hiding this comment.
Community review, not a merge gate.
Traced _normalize_provider_data_key in src/ogx/core/routers/inference.py and its call site in _get_model_provider, plus the companion error-message change in openai_mixin.py. The core idea is sound: let a client authenticate a specific named provider instance via {provider_id}_api_key without knowing the backend type's internal key name (e.g. vllm_api_token), matched case-insensitively so the client doesn't need to reproduce the provider id's exact casing, while leaving the legacy type-keyed field untouched for existing callers. The refactor of _get_model_provider to a single return path (calling normalize before returning in both the model-lookup and fallback branches) is a clean way to guarantee the normalization always runs regardless of which branch resolved the provider.
The case-insensitive match is careful about one thing I'd have expected to be a bug and isn't: test_normalize_provider_data_key_ignores_legacy_key_casing confirms a differently-cased legacy key (VLLM_API_TOKEN) is not mistaken for the client key, since the match target is built from provider_id, not from impl_key. Good, that's the kind of case that's easy to get wrong.
The tests are solid: they cover no-provider-data, absent client key, absent impl key, client-key-overrides-legacy-value precedence, case variations, and an end-to-end check that the normalized key actually reaches the constructed AsyncOpenAI client's api_key. _get_model_provider_normalizes_provider_data_key and its "without provider data" counterpart also verify the router-level integration, not just the helper in isolation.
One thing I didn't verify: PROVIDER_DATA_VAR is a contextvar re-set mid-request with {**provider_data, impl_key: ...}. That's safe as long as each request runs in its own context (standard for ASGI frameworks), but I didn't trace how this specific framework propagates context across the background-task path mentioned earlier in the same file (_log_background_task_error), so I can't personally rule out a cross-request leak in that specific path — it's probably fine, just outside what I traced.
|
@VANDRANKI thanks. which model and what was your prompt? |
|
To be clear: my review was a code-level trace of |
|
@VANDRANKI ok, any changes you'd suggest? |
|
No changes from what I traced. My review covered The one thing I flagged as unverified, not as a problem: I didn't trace how If you still want the live Chat Playground check, that needs someone who can actually run a model through it. I only traced code, I didn't execute anything. |
2 participants
Per-request credentials for inference providers are passed via the X-OGX-Provider-Data header, keyed by each provider's internal provider_data_api_key_field. That field is shaped by the backend type (e.g. vllm_api_token for a remote::vllm provider), so the header leaked the provider type to the client. A user had to know that a provider was backed by vLLM in order to send the correct per-request credential key, which defeats the point of addressing a provider purely by its name. It also made it impossible to run two providers of the same type and give each a distinct credential, since both shared the same type-shaped key.
Resolve the leak by normalizing the key in the inference router. The router maps the client-facing {provider_id}_api_key onto the provider's internal provider_data_api_key_field on the request provider-data context before dispatching. Clients authenticate a named provider without knowing its backend type, and same-type providers can each be keyed by their own provider id (e.g. two remote::vllm providers using providerA_api_key and providerB_api_key).
The mapping lives in the router rather than in the provider implementations so each provider's per-type provider-data validator is left untouched: it still observes the internal key it already knows. The legacy type-shaped key keeps working unchanged for backward compatibility, and when both are present the provider-id key takes precedence. The missing-key error in the OpenAI mixin now references {provider_id}_api_key instead of the backend type, closing the remaining leak.
Scope: the inference router, covering the OpenAI-mixin inference providers. Non-OpenAI remote providers and other router surfaces are follow-up work.
Adds unit tests for the normalization helper, the router call-site, and the error-message branches, plus integration tests exercising the provider-id key against a vllm provider, and documents the convention in the "Using OGX as a Library" guide.