feat: remove direct key bypass from HTTP gateway and Go SDK#3241
Merged
feat: remove direct key bypass from HTTP gateway and Go SDK#3241
Conversation
|
|
Collaborator
Author
18 tasks
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR removes Direct Key Bypass (allow_direct_keys / BifrostContextKeyDirectKey) across config, core key-selection, HTTP handlers, DB schema/migrations, tests, Helm, UI, and docs; key resolution now uses Bifrost-managed provider keys and virtual keys exclusively. ChangesDirect Key Bypass Removal
sequenceDiagram Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.1)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
Contributor
Confidence Score: 4/5Safe to merge for the vast majority of deployments; the one defect is in the hash-recompute migration, which causes a one-time spurious config reload at upgrade for any deployment that has The feature removal is thorough and consistent across the core, transports, UI, and docs. The only defect is in framework/configstore/migrations.go — the hash-recompute Important Files Changed
Reviews (4): Last reviewed commit: "refactor: remove support for direct api ..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/bifrost.go (1)
7147-7180:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHonor
APIKeyID/APIKeyNamepinning in the multi-key helpers.After removing
DirectKey, these helpers now always return every eligible key.selectKeyFromProviderForModelWithPool()still respectsschemas.BifrostContextKeyAPIKeyID/schemas.BifrostContextKeyAPIKeyName, butListModelsRequestand the multi-key batch/file/container/cached-content paths bypass that function and therefore ignore the replacement pinning mechanism. With multiple managed keys configured, that can route list/retrieve/delete-style operations to the wrong provider account.Suggested fix
+func filterPinnedKeys(ctx *schemas.BifrostContext, keys []schemas.Key, providerKey schemas.ModelProvider, model string) ([]schemas.Key, error) { + if ctx == nil { + return keys, nil + } + + if keyID, ok := ctx.Value(schemas.BifrostContextKeyAPIKeyID).(string); ok { + keyID = strings.TrimSpace(keyID) + if keyID != "" { + for _, key := range keys { + if key.ID == keyID { + return []schemas.Key{key}, nil + } + } + return nil, fmt.Errorf("no supported key found with id %q for provider: %v and model: %s", keyID, providerKey, model) + } + } + + if keyName, ok := ctx.Value(schemas.BifrostContextKeyAPIKeyName).(string); ok { + keyName = strings.TrimSpace(keyName) + if keyName != "" { + for _, key := range keys { + if key.Name == keyName { + return []schemas.Key{key}, nil + } + } + return nil, fmt.Errorf("no supported key found with name %q for provider: %v and model: %s", keyName, providerKey, model) + } + } + + return keys, nil +}Then apply it before the final
len(...) == 0/ return in both helpers.Also applies to: 7185-7249
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 7147 - 7180, getAllSupportedKeys (and the sibling multi-key helper around lines 7185-7249) currently returns all eligible keys and ignores pinning values in the Bifrost context; update both helpers to read schemas.BifrostContextKeyAPIKeyID and schemas.BifrostContextKeyAPIKeyName from the provided ctx and filter/singularize supportedKeys to only the pinned key(s) when those context values are present (same semantics used by selectKeyFromProviderForModelWithPool), then proceed with the existing empty-check and return logic so ListModelsRequest and the batch/file/container/cached-content paths honor APIKeyID/APIKeyName pinning.
🧹 Nitpick comments (1)
docs/migration-guides/v1.5.0.mdx (1)
702-752: ⚡ Quick winUse Mintlify tabs for Web UI / API / config.json in this migration section.
This block mixes config/API migration paths in prose; please format it with the required Mintlify tab structure for consistency and discoverability.
As per coding guidelines,
docs/**/*.mdx: Mintlify MDX documentation must have Web UI / API / config.json tabs; validate config.json examples against transports/config.schema.json.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/migration-guides/v1.5.0.mdx` around lines 702 - 752, This migration section under the "How to update" header mixes Web UI, API and config.json guidance; split the block into Mintlify tabs labeled "Web UI", "API" and "config.json" and move the relevant paragraphs into each tab (the config.json diff snippet and the note about validating against transports/config.schema.json belong in the "config.json" tab; the PUT /api/config and provider/key API guidance belong in "API"; the Go SDK code samples and BifrostContextKeyDirectKey / BifrostContextKeyAPIKeyName examples belong in "Web UI" or "API" as appropriate). Ensure the config.json example is validated against transports/config.schema.json and keep the unique identifiers (BifrostContextKeyDirectKey, BifrostContextKeyAPIKeyName, BifrostContextKeyAPIKeyID, and the config.json snippet) intact so reviewers can find and verify the changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/migration-guides/v1.5.0.mdx`:
- Line 678: Rename the duplicated "Breaking Change 14: Direct Key Bypass Removed
(HTTP Gateway and Go SDK)" header to the next sequential number (e.g., "Breaking
Change 15") and update any internal references that point to "Breaking Change
14" in the same document (including the two troubleshooting links that reference
this section and the duplicate occurrences around the later block at lines
~902-907) so they point to the new number; specifically search for the exact
header text "Breaking Change 14: Direct Key Bypass Removed (HTTP Gateway and Go
SDK)" and the troubleshooting references that mention "Breaking Change 14" and
change them to the new header label consistently across the file.
---
Outside diff comments:
In `@core/bifrost.go`:
- Around line 7147-7180: getAllSupportedKeys (and the sibling multi-key helper
around lines 7185-7249) currently returns all eligible keys and ignores pinning
values in the Bifrost context; update both helpers to read
schemas.BifrostContextKeyAPIKeyID and schemas.BifrostContextKeyAPIKeyName from
the provided ctx and filter/singularize supportedKeys to only the pinned key(s)
when those context values are present (same semantics used by
selectKeyFromProviderForModelWithPool), then proceed with the existing
empty-check and return logic so ListModelsRequest and the
batch/file/container/cached-content paths honor APIKeyID/APIKeyName pinning.
---
Nitpick comments:
In `@docs/migration-guides/v1.5.0.mdx`:
- Around line 702-752: This migration section under the "How to update" header
mixes Web UI, API and config.json guidance; split the block into Mintlify tabs
labeled "Web UI", "API" and "config.json" and move the relevant paragraphs into
each tab (the config.json diff snippet and the note about validating against
transports/config.schema.json belong in the "config.json" tab; the PUT
/api/config and provider/key API guidance belong in "API"; the Go SDK code
samples and BifrostContextKeyDirectKey / BifrostContextKeyAPIKeyName examples
belong in "Web UI" or "API" as appropriate). Ensure the config.json example is
validated against transports/config.schema.json and keep the unique identifiers
(BifrostContextKeyDirectKey, BifrostContextKeyAPIKeyName,
BifrostContextKeyAPIKeyID, and the config.json snippet) intact so reviewers can
find and verify the changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8e2a599f-d1e8-4e76-a196-1c7c7b68d433
📒 Files selected for processing (66)
.github/workflows/configs/withpostgresmcpclientsinconfig/config.jsoncore/bifrost.gocore/internal/llmtests/account.gocore/internal/llmtests/list_models.gocore/providers/utils/utils.gocore/schemas/bifrost.gocore/schemas/context.godocs/deployment-guides/config-json.mdxdocs/deployment-guides/config-json/client.mdxdocs/deployment-guides/config-json/schema-reference.mdxdocs/deployment-guides/helm.mdxdocs/deployment-guides/helm/client.mdxdocs/deployment-guides/helm/values.mdxdocs/enterprise/migration-guides/v1.4.0.mdxdocs/features/governance/virtual-keys.mdxdocs/features/keys-management.mdxdocs/features/observability/default.mdxdocs/integrations/anthropic-sdk/overview.mdxdocs/integrations/bedrock-sdk/overview.mdxdocs/integrations/genai-sdk/overview.mdxdocs/integrations/langchain-sdk.mdxdocs/integrations/litellm-sdk.mdxdocs/integrations/openai-sdk/overview.mdxdocs/integrations/pydanticai-sdk.mdxdocs/migration-guides/v1.5.0.mdxdocs/openapi/openapi.jsondocs/openapi/schemas/management/config.yamldocs/providers/request-options.mdxdocs/quickstart/go-sdk/context-keys.mdxexamples/configs/withpostgresmcpclientsinconfig/config.jsonexamples/configs/withprompushgateway/config.jsonexamples/configs/withvirtualkeys/config.jsonexamples/dockers/data/config.jsonexamples/k8s/examples/values-client-configs.yamlframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values-examples/providers-and-virtual-keys.yamlhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamlrecipes/values-local-k8s.yamltests/governance/config.jsontests/integrations/python/config.jsontests/integrations/typescript/config.jsontransports/bifrost-http/handlers/config.gotransports/bifrost-http/handlers/webrtc_realtime.gotransports/bifrost-http/handlers/webrtc_realtime_test.gotransports/bifrost-http/handlers/wsresponses.gotransports/bifrost-http/handlers/wsresponses_test.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/router_large_payload_test.gotransports/bifrost-http/integrations/router_test.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/lib/config_test.gotransports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/config.schema.jsonui/app/workspace/config/views/securityView.tsxui/lib/types/config.tsui/lib/types/schemas.ts
💤 Files with no reviewable changes (49)
- examples/configs/withpostgresmcpclientsinconfig/config.json
- .github/workflows/configs/withpostgresmcpclientsinconfig/config.json
- examples/configs/withvirtualkeys/config.json
- docs/features/governance/virtual-keys.mdx
- docs/integrations/litellm-sdk.mdx
- docs/integrations/pydanticai-sdk.mdx
- tests/integrations/python/config.json
- docs/deployment-guides/config-json/client.mdx
- docs/deployment-guides/config-json.mdx
- docs/deployment-guides/config-json/schema-reference.mdx
- docs/integrations/langchain-sdk.mdx
- framework/configstore/tables/clientconfig.go
- transports/bifrost-http/handlers/config.go
- tests/integrations/typescript/config.json
- tests/governance/config.json
- examples/configs/withprompushgateway/config.json
- docs/openapi/openapi.json
- docs/integrations/anthropic-sdk/overview.mdx
- ui/lib/types/config.ts
- docs/deployment-guides/helm.mdx
- docs/providers/request-options.mdx
- core/schemas/context.go
- helm-charts/bifrost/values.schema.json
- core/internal/llmtests/account.go
- helm-charts/bifrost/values.yaml
- recipes/values-local-k8s.yaml
- docs/integrations/openai-sdk/overview.mdx
- framework/configstore/rdb_test.go
- core/schemas/bifrost.go
- transports/bifrost-http/handlers/webrtc_realtime_test.go
- examples/k8s/examples/values-client-configs.yaml
- docs/deployment-guides/helm/values.mdx
- transports/bifrost-http/handlers/webrtc_realtime.go
- docs/integrations/genai-sdk/overview.mdx
- ui/lib/types/schemas.ts
- transports/bifrost-http/lib/config.go
- helm-charts/bifrost/templates/_helpers.tpl
- transports/bifrost-http/handlers/wsresponses.go
- framework/configstore/rdb.go
- helm-charts/bifrost/values-examples/providers-and-virtual-keys.yaml
- transports/bifrost-http/integrations/router.go
- framework/configstore/clientconfig.go
- docs/openapi/schemas/management/config.yaml
- docs/integrations/bedrock-sdk/overview.mdx
- transports/config.schema.json
- transports/bifrost-http/lib/config_test.go
- docs/quickstart/go-sdk/context-keys.mdx
- transports/bifrost-http/integrations/openai.go
- transports/bifrost-http/lib/ctx.go
Pratham-Mishra04
changed the base branch from
05-06-refactor_rename_semantic_cache_plugin_to_local_cache_and_refactor_internal_wiring
to
graphite-base/3241
Pratham-Mishra04
force-pushed
the
graphite-base/3241
branch
from
a862d6e to
ecbfec1
Compare
Pratham-Mishra04
force-pushed
the
05-05-refactor_remove_support_for_direct_api_keys
branch
from
062073b to
a9d6459
Compare
This was referenced May 6, 2026
Merged
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/bifrost.go (1)
5720-5776:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve APIKeyID/APIKeyName pinning in the
ListModelsand multi-key branches.Only
selectKeyFromProviderForModelWithPoolappliesBifrostContextKeyAPIKeyID/BifrostContextKeyAPIKeyName. After this split,ListModelsRequestand the multi-key batch/file/container/cached-content paths bypass that filter entirely, so the documented migration away from direct-key support stops working for those endpoints.♻️ Suggested direction
+filterPinnedKeys := func(keys []schemas.Key) ([]schemas.Key, error) { + if keyID, ok := req.Context.Value(schemas.BifrostContextKeyAPIKeyID).(string); ok && strings.TrimSpace(keyID) != "" { + for _, key := range keys { + if key.ID == keyID { + return []schemas.Key{key}, nil + } + } + return nil, fmt.Errorf("no supported key found with id %q for provider: %v and model: %s", keyID, provider.GetProviderKey(), model) + } + if keyName, ok := req.Context.Value(schemas.BifrostContextKeyAPIKeyName).(string); ok && strings.TrimSpace(keyName) != "" { + for _, key := range keys { + if key.Name == keyName { + return []schemas.Key{key}, nil + } + } + return nil, fmt.Errorf("no supported key found with name %q for provider: %v and model: %s", keyName, provider.GetProviderKey(), model) + } + return keys, nil +} + if req.RequestType == schemas.ListModelsRequest { keys, err = bifrost.getAllSupportedKeys(req.Context, provider.GetProviderKey(), baseProvider) + if err == nil { + keys, err = filterPinnedKeys(keys) + } ... } else if isMultiKeyBatchOp || isMultiKeyFileOp || isMultiKeyContainerOp || isMultiKeyCachedContentOp { ... keys, err = bifrost.getKeysForBatchAndFileOps(req.Context, provider.GetProviderKey(), baseProvider, modelPtr, isMultiKeyBatchOp) + if err == nil { + keys, err = filterPinnedKeys(keys) + } ... }As per coding guidelines,
docs/features/keys-management.mdxstates: "per-request pinning via Go SDK should use APIKeyID/Name; bypass is no longer supported."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 5720 - 5776, ListModelsRequest and the multi-key branches bypass per-request API key pinning, so ensure the BifrostContextKeyAPIKeyID / BifrostContextKeyAPIKeyName pinning is applied before calling getAllSupportedKeys and getKeysForBatchAndFileOps; either call selectKeyFromProviderForModelWithPool first to get a context/pinned key or copy the same pinning/filter logic into the branches and inject the pinned key into req.Context (or pass a pinnedContext) so getAllSupportedKeys(provider.GetProviderKey(), ...) and getKeysForBatchAndFileOps(...) receive the pinned key context; reference functions/values: getAllSupportedKeys, getKeysForBatchAndFileOps, selectKeyFromProviderForModelWithPool, BifrostContextKeyAPIKeyID, BifrostContextKeyAPIKeyName.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/migration-guides/v1.5.0.mdx`:
- Around line 548-599: Split the "How to update" section into Mintlify tabs
labeled "Web UI", "API", and "config.json" and move the relevant migration steps
into each tab (UI guidance → Web UI; PUT /api/config and provider/key migration
→ API; the JSON example and note about removed allow_direct_keys → config.json).
In the config.json tab replace the current example with one that validates
against transports/config.schema.json (remove the dropped allow_direct_keys
field and ensure keys/fields match the schema). In the API tab keep the PUT
/api/config and provider/key instructions, and in the SDK/Go guidance mention
the exact constants to change (replace schemas.BifrostContextKeyDirectKey with
schemas.BifrostContextKeyAPIKeyName or schemas.BifrostContextKeyAPIKeyID and
note BifrostContextKeySkipKeySelection remains) so readers can find the code
paths. Ensure tab markup follows Mintlify MDX conventions used in docs/**/*.mdx.
In `@framework/configstore/migrations.go`:
- Around line 1072-1105: The rebuilt ClientConfig used to recompute the hash
omits the compat flags, so GenerateClientConfigHash() (which includes Compat.*
in its hash) produces a stale value; update the migration to copy the compat
fields from the existing cc into the new ClientConfig (i.e., set the
Compat-related fields on the clientConfig struct before calling
GenerateClientConfigHash()), then proceed with the
tx.Model(&cc).Update("config_hash", newHash) as before so deployments with
compat flags produce the correct hash.
---
Outside diff comments:
In `@core/bifrost.go`:
- Around line 5720-5776: ListModelsRequest and the multi-key branches bypass
per-request API key pinning, so ensure the BifrostContextKeyAPIKeyID /
BifrostContextKeyAPIKeyName pinning is applied before calling
getAllSupportedKeys and getKeysForBatchAndFileOps; either call
selectKeyFromProviderForModelWithPool first to get a context/pinned key or copy
the same pinning/filter logic into the branches and inject the pinned key into
req.Context (or pass a pinnedContext) so
getAllSupportedKeys(provider.GetProviderKey(), ...) and
getKeysForBatchAndFileOps(...) receive the pinned key context; reference
functions/values: getAllSupportedKeys, getKeysForBatchAndFileOps,
selectKeyFromProviderForModelWithPool, BifrostContextKeyAPIKeyID,
BifrostContextKeyAPIKeyName.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d4272b0-f0dc-4639-9081-5410cd95f532
📒 Files selected for processing (66)
.github/workflows/configs/withpostgresmcpclientsinconfig/config.jsoncore/bifrost.gocore/internal/llmtests/account.gocore/internal/llmtests/list_models.gocore/providers/utils/utils.gocore/schemas/bifrost.gocore/schemas/context.godocs/deployment-guides/config-json.mdxdocs/deployment-guides/config-json/client.mdxdocs/deployment-guides/config-json/schema-reference.mdxdocs/deployment-guides/helm.mdxdocs/deployment-guides/helm/client.mdxdocs/deployment-guides/helm/values.mdxdocs/enterprise/migration-guides/v1.4.0.mdxdocs/features/governance/virtual-keys.mdxdocs/features/keys-management.mdxdocs/features/observability/default.mdxdocs/integrations/anthropic-sdk/overview.mdxdocs/integrations/bedrock-sdk/overview.mdxdocs/integrations/genai-sdk/overview.mdxdocs/integrations/langchain-sdk.mdxdocs/integrations/litellm-sdk.mdxdocs/integrations/openai-sdk/overview.mdxdocs/integrations/pydanticai-sdk.mdxdocs/migration-guides/v1.5.0.mdxdocs/openapi/openapi.jsondocs/openapi/schemas/management/config.yamldocs/providers/request-options.mdxdocs/quickstart/go-sdk/context-keys.mdxexamples/configs/withpostgresmcpclientsinconfig/config.jsonexamples/configs/withprompushgateway/config.jsonexamples/configs/withvirtualkeys/config.jsonexamples/dockers/data/config.jsonexamples/k8s/examples/values-client-configs.yamlframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values-examples/providers-and-virtual-keys.yamlhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamlrecipes/values-local-k8s.yamltests/governance/config.jsontests/integrations/python/config.jsontests/integrations/typescript/config.jsontransports/bifrost-http/handlers/config.gotransports/bifrost-http/handlers/webrtc_realtime.gotransports/bifrost-http/handlers/webrtc_realtime_test.gotransports/bifrost-http/handlers/wsresponses.gotransports/bifrost-http/handlers/wsresponses_test.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/router_large_payload_test.gotransports/bifrost-http/integrations/router_test.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/lib/config_test.gotransports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/config.schema.jsonui/app/workspace/config/views/securityView.tsxui/lib/types/config.tsui/lib/types/schemas.ts
💤 Files with no reviewable changes (49)
- transports/bifrost-http/integrations/router.go
- recipes/values-local-k8s.yaml
- docs/integrations/litellm-sdk.mdx
- docs/deployment-guides/helm/values.mdx
- docs/features/governance/virtual-keys.mdx
- core/internal/llmtests/account.go
- docs/deployment-guides/helm.mdx
- docs/integrations/genai-sdk/overview.mdx
- docs/deployment-guides/config-json/schema-reference.mdx
- docs/openapi/schemas/management/config.yaml
- helm-charts/bifrost/values.schema.json
- helm-charts/bifrost/templates/_helpers.tpl
- tests/integrations/typescript/config.json
- core/schemas/context.go
- framework/configstore/tables/clientconfig.go
- tests/integrations/python/config.json
- docs/integrations/pydanticai-sdk.mdx
- docs/integrations/langchain-sdk.mdx
- docs/providers/request-options.mdx
- docs/quickstart/go-sdk/context-keys.mdx
- docs/deployment-guides/config-json.mdx
- helm-charts/bifrost/values-examples/providers-and-virtual-keys.yaml
- docs/integrations/bedrock-sdk/overview.mdx
- framework/configstore/rdb_test.go
- ui/lib/types/config.ts
- tests/governance/config.json
- docs/integrations/openai-sdk/overview.mdx
- transports/bifrost-http/handlers/config.go
- examples/configs/withprompushgateway/config.json
- transports/bifrost-http/lib/config.go
- framework/configstore/clientconfig.go
- transports/bifrost-http/lib/config_test.go
- .github/workflows/configs/withpostgresmcpclientsinconfig/config.json
- docs/deployment-guides/config-json/client.mdx
- examples/configs/withvirtualkeys/config.json
- transports/bifrost-http/handlers/webrtc_realtime.go
- docs/openapi/openapi.json
- examples/k8s/examples/values-client-configs.yaml
- transports/bifrost-http/handlers/webrtc_realtime_test.go
- framework/configstore/rdb.go
- transports/config.schema.json
- core/schemas/bifrost.go
- helm-charts/bifrost/values.yaml
- examples/configs/withpostgresmcpclientsinconfig/config.json
- transports/bifrost-http/lib/ctx.go
- transports/bifrost-http/integrations/openai.go
- ui/lib/types/schemas.ts
- docs/integrations/anthropic-sdk/overview.mdx
- transports/bifrost-http/handlers/wsresponses.go
🚧 Files skipped from review as they are similar to previous changes (2)
- transports/bifrost-http/integrations/bedrock_test.go
- transports/bifrost-http/lib/ctx_test.go
Pratham-Mishra04
force-pushed
the
05-05-refactor_remove_support_for_direct_api_keys
branch
from
a9d6459 to
2525d89
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
core/bifrost.go (3)
7147-7180:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve
APIKeyID/APIKeyNamepinning on the fan-out helpers.After removing the direct-key path, explicit key pinning is the remaining way to scope a request to one provider account, but these helpers now always start from the full provider key set. That means
ListModels, batch/file/container/cached-content operations can ignore a pinned key and fan out across every eligible account.A small shared filter for
schemas.BifrostContextKeyAPIKeyID/schemas.BifrostContextKeyAPIKeyNamebefore the existing enabled/model checks would keep the stack’s migration path intact.Also applies to: 7185-7248
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 7147 - 7180, The getAllSupportedKeys function currently fans out across all provider keys; add a pre-filter that honors pinned API key context values by checking ctx.Context for schemas.BifrostContextKeyAPIKeyID and schemas.BifrostContextKeyAPIKeyName and, if present, restrict the iteration to only the matching key(s) before applying the Enabled and validateKey checks (i.e., immediately after obtaining keys and before the loop/Enabled check). Apply the same pre-filter logic to the sibling helper(s) around lines 7185-7248 so any request-scoped pinning via those BifrostContext keys limits the returned supportedKeys and prevents unintended fan-out.
7265-7267:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate
SkipKeySelectiononbaseProviderType, not the custom provider name.For custom providers,
providerKeyis the custom alias, soisKeySkippingAllowed(providerKey)can reject keyless requests even when the underlying base provider supports them. With direct-key bypass removed, that turns valid custom keyless routes into hard failures.💡 Suggested fix
- if skipKeySelection, ok := ctx.Value(schemas.BifrostContextKeySkipKeySelection).(bool); ok && skipKeySelection && isKeySkippingAllowed(providerKey) { + if skipKeySelection, ok := ctx.Value(schemas.BifrostContextKeySkipKeySelection).(bool); ok && skipKeySelection && isKeySkippingAllowed(baseProviderType) { return []schemas.Key{}, false, nil }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 7265 - 7267, The SkipKeySelection gate is currently checking isKeySkippingAllowed(providerKey) (which is the custom alias) and can wrongly block custom providers; change that check to use the underlying base provider type (e.g. baseProviderType) instead of providerKey so keyless routes for custom providers follow the base provider's policy; update the conditional around ctx.Value(schemas.BifrostContextKeySkipKeySelection) to call isKeySkippingAllowed(baseProviderType) and keep the existing return of []schemas.Key{}, false, nil when allowed.
5310-5314:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClear key-selection metadata when a request is running keyless.
When
keyProvider == nil, this loop leavesschemas.BifrostContextKeyAttemptTrail,schemas.BifrostContextKeySelectedKeyID, andschemas.BifrostContextKeySelectedKeyNameuntouched. On a reused context, a keyless request can therefore inherit the previous request’s key attribution and leak wrong data into logs, traces, and plugins.Reset these fields on attempt 0 even when selection is skipped. Based on learnings,
(*schemas.BifrostContext).SetValue(key, value)mutates the shared context in place, so stale values remain visible to later requests.Also applies to: 5359-5360, 5592-5597
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 5310 - 5314, The context reset only runs when keyProvider != nil, so when keyProvider == nil a reused bifrost.ctx can retain stale values; update the logic around attempts == 0 (where ctx.SetValue is called) to also clear schemas.BifrostContextKeyAttemptTrail, schemas.BifrostContextKeySelectedKeyID, and schemas.BifrostContextKeySelectedKeyName even when keyProvider is nil (i.e., on attempts == 0 always). Locate uses of attempts, keyProvider and ctx.SetValue in core/bifrost.go (the blocks at ~5310, ~5359-5360, and ~5592-5597) and add unconditional SetValue calls to reset those three keys to empty values (empty slice for attempt trail and empty string/nil for selected key id/name) so keyless requests cannot inherit previous request metadata.
🧹 Nitpick comments (1)
transports/bifrost-http/lib/ctx_test.go (1)
143-169: ⚡ Quick winAdd explicit direct-header checks for provider key headers in wildcard mode.
Given this PR’s breaking change, this test should also assert that
authorization,x-api-key, andx-goog-api-keyare blocked on the direct-forwarding path (not justproxy-authorization/cookie/host/connection).Proposed test hardening
ctx := &fasthttp.RequestCtx{} // Direct headers (not prefixed with x-bf-eh-) ctx.Request.Header.Set("custom-header", "allowed-value") ctx.Request.Header.Set("anthropic-beta", "some-beta-feature") // Security headers sent directly — should be blocked +ctx.Request.Header.Set("authorization", "Bearer raw-provider-key") +ctx.Request.Header.Set("x-api-key", "raw-provider-key") +ctx.Request.Header.Set("x-goog-api-key", "raw-provider-key") ctx.Request.Header.Set("proxy-authorization", "should-be-blocked") ... -directSecurityHeaders := []string{"proxy-authorization", "cookie", "host", "connection"} +directSecurityHeaders := []string{ + "authorization", + "x-api-key", + "x-goog-api-key", + "proxy-authorization", + "cookie", + "host", + "connection", +} for _, h := range directSecurityHeaders { if _, ok := extraHeaders[h]; ok { t.Errorf("expected security header %q to be blocked in direct forwarding even with * allowlist", h) } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@transports/bifrost-http/lib/ctx_test.go` around lines 143 - 169, The test currently asserts direct-forwarding blocks for proxy-authorization, cookie, host, and connection but misses provider key headers; update the test that calls ConvertToBifrostContext (using bifrostCtx and extracting extraHeaders via schemas.BifrostContextKeyExtraHeaders) to also check that "authorization", "x-api-key", and "x-goog-api-key" are not present in extraHeaders when wildcard allowlist is used—add these three header names to the directSecurityHeaders slice (or a separate assertion loop) so the test fails if any of those provider keys slip through the direct forwarding path.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/providers/utils/utils.go`:
- Around line 2793-2794: The current check returns defaultProvider whenever
ctx.Value(schemas.BifrostContextKeySkipKeySelection) is non-nil, which
incorrectly treats an explicit false as skip; update the logic in the function
that references BifrostContextKeySkipKeySelection to type-assert the context
value to a bool and only short-circuit to return defaultProvider when that
asserted bool is true (treat non-bool or missing values as false so the
availableProviders branch runs normally).
In `@docs/enterprise/migration-guides/v1.4.0.mdx`:
- Around line 161-163: Update the checklist sentence under the Step titled
"Apply the OSS v1.5.0 migration steps" to split HTTP and Go SDK migration paths:
for Go SDK/in‑process callers, instruct migrating from
BifrostContextKeyDirectKey to
BifrostContextKeyAPIKeyID/BifrostContextKeyAPIKeyName; for HTTP/header callers,
instruct following the OSS virtual-key or managed-provider-key path (migrating
header-key usage and removing allow_direct_keys) and point to the appropriate
OSS v1.5.0 guidance for provider key and VK configuration changes.
In `@docs/features/keys-management.mdx`:
- Line 172: The wording incorrectly implies that per-request pinning via
schemas.BifrostContextKeyAPIKeyID and schemas.BifrostContextKeyAPIKeyName only
works with keys created through the providers API; update the sentence to
clarify these context keys work with any Bifrost-managed key source (including
custom Account implementations), not just keys created via POST
/api/providers/{provider}/keys, and adjust the example text to avoid suggesting
the providers API is required for per-request pinning.
In `@framework/configstore/migrations.go`:
- Around line 1069-1107: The recompute loop builds a ClientConfig from cc but
uses cc.RoutingChainMaxDepth directly, causing upgraded rows that still have 0
to produce a different hash; update the construction in the loop (same place
that calls ClientConfig.GenerateClientConfigHash) to normalize
cc.RoutingChainMaxDepth to the legacy default (the same normalization/migration
logic used by migrationAddRoutingChainMaxDepthColumn) before assigning into
ClientConfig.RoutingChainMaxDepth so the hash remains stable for rows that read
back as 0.
---
Outside diff comments:
In `@core/bifrost.go`:
- Around line 7147-7180: The getAllSupportedKeys function currently fans out
across all provider keys; add a pre-filter that honors pinned API key context
values by checking ctx.Context for schemas.BifrostContextKeyAPIKeyID and
schemas.BifrostContextKeyAPIKeyName and, if present, restrict the iteration to
only the matching key(s) before applying the Enabled and validateKey checks
(i.e., immediately after obtaining keys and before the loop/Enabled check).
Apply the same pre-filter logic to the sibling helper(s) around lines 7185-7248
so any request-scoped pinning via those BifrostContext keys limits the returned
supportedKeys and prevents unintended fan-out.
- Around line 7265-7267: The SkipKeySelection gate is currently checking
isKeySkippingAllowed(providerKey) (which is the custom alias) and can wrongly
block custom providers; change that check to use the underlying base provider
type (e.g. baseProviderType) instead of providerKey so keyless routes for custom
providers follow the base provider's policy; update the conditional around
ctx.Value(schemas.BifrostContextKeySkipKeySelection) to call
isKeySkippingAllowed(baseProviderType) and keep the existing return of
[]schemas.Key{}, false, nil when allowed.
- Around line 5310-5314: The context reset only runs when keyProvider != nil, so
when keyProvider == nil a reused bifrost.ctx can retain stale values; update the
logic around attempts == 0 (where ctx.SetValue is called) to also clear
schemas.BifrostContextKeyAttemptTrail, schemas.BifrostContextKeySelectedKeyID,
and schemas.BifrostContextKeySelectedKeyName even when keyProvider is nil (i.e.,
on attempts == 0 always). Locate uses of attempts, keyProvider and ctx.SetValue
in core/bifrost.go (the blocks at ~5310, ~5359-5360, and ~5592-5597) and add
unconditional SetValue calls to reset those three keys to empty values (empty
slice for attempt trail and empty string/nil for selected key id/name) so
keyless requests cannot inherit previous request metadata.
---
Nitpick comments:
In `@transports/bifrost-http/lib/ctx_test.go`:
- Around line 143-169: The test currently asserts direct-forwarding blocks for
proxy-authorization, cookie, host, and connection but misses provider key
headers; update the test that calls ConvertToBifrostContext (using bifrostCtx
and extracting extraHeaders via schemas.BifrostContextKeyExtraHeaders) to also
check that "authorization", "x-api-key", and "x-goog-api-key" are not present in
extraHeaders when wildcard allowlist is used—add these three header names to the
directSecurityHeaders slice (or a separate assertion loop) so the test fails if
any of those provider keys slip through the direct forwarding path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 89e9b0f3-c113-4e9d-9702-8e09c1d88e99
📒 Files selected for processing (66)
.github/workflows/configs/withpostgresmcpclientsinconfig/config.jsoncore/bifrost.gocore/internal/llmtests/account.gocore/internal/llmtests/list_models.gocore/providers/utils/utils.gocore/schemas/bifrost.gocore/schemas/context.godocs/deployment-guides/config-json.mdxdocs/deployment-guides/config-json/client.mdxdocs/deployment-guides/config-json/schema-reference.mdxdocs/deployment-guides/helm.mdxdocs/deployment-guides/helm/client.mdxdocs/deployment-guides/helm/values.mdxdocs/enterprise/migration-guides/v1.4.0.mdxdocs/features/governance/virtual-keys.mdxdocs/features/keys-management.mdxdocs/features/observability/default.mdxdocs/integrations/anthropic-sdk/overview.mdxdocs/integrations/bedrock-sdk/overview.mdxdocs/integrations/genai-sdk/overview.mdxdocs/integrations/langchain-sdk.mdxdocs/integrations/litellm-sdk.mdxdocs/integrations/openai-sdk/overview.mdxdocs/integrations/pydanticai-sdk.mdxdocs/migration-guides/v1.5.0.mdxdocs/openapi/openapi.jsondocs/openapi/schemas/management/config.yamldocs/providers/request-options.mdxdocs/quickstart/go-sdk/context-keys.mdxexamples/configs/withpostgresmcpclientsinconfig/config.jsonexamples/configs/withprompushgateway/config.jsonexamples/configs/withvirtualkeys/config.jsonexamples/dockers/data/config.jsonexamples/k8s/examples/values-client-configs.yamlframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values-examples/providers-and-virtual-keys.yamlhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamlrecipes/values-local-k8s.yamltests/governance/config.jsontests/integrations/python/config.jsontests/integrations/typescript/config.jsontransports/bifrost-http/handlers/config.gotransports/bifrost-http/handlers/webrtc_realtime.gotransports/bifrost-http/handlers/webrtc_realtime_test.gotransports/bifrost-http/handlers/wsresponses.gotransports/bifrost-http/handlers/wsresponses_test.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/router_large_payload_test.gotransports/bifrost-http/integrations/router_test.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/lib/config_test.gotransports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/config.schema.jsonui/app/workspace/config/views/securityView.tsxui/lib/types/config.tsui/lib/types/schemas.ts
💤 Files with no reviewable changes (49)
- core/schemas/context.go
- examples/configs/withpostgresmcpclientsinconfig/config.json
- ui/lib/types/config.ts
- docs/deployment-guides/config-json.mdx
- examples/configs/withprompushgateway/config.json
- docs/integrations/langchain-sdk.mdx
- helm-charts/bifrost/values.schema.json
- docs/openapi/openapi.json
- docs/integrations/genai-sdk/overview.mdx
- recipes/values-local-k8s.yaml
- docs/deployment-guides/helm/values.mdx
- ui/lib/types/schemas.ts
- docs/providers/request-options.mdx
- docs/openapi/schemas/management/config.yaml
- tests/integrations/typescript/config.json
- docs/integrations/pydanticai-sdk.mdx
- framework/configstore/rdb_test.go
- .github/workflows/configs/withpostgresmcpclientsinconfig/config.json
- tests/governance/config.json
- examples/k8s/examples/values-client-configs.yaml
- docs/quickstart/go-sdk/context-keys.mdx
- core/internal/llmtests/account.go
- examples/configs/withvirtualkeys/config.json
- docs/integrations/bedrock-sdk/overview.mdx
- docs/deployment-guides/config-json/schema-reference.mdx
- docs/integrations/litellm-sdk.mdx
- transports/bifrost-http/handlers/wsresponses.go
- transports/config.schema.json
- transports/bifrost-http/lib/ctx.go
- docs/integrations/openai-sdk/overview.mdx
- docs/integrations/anthropic-sdk/overview.mdx
- helm-charts/bifrost/values.yaml
- transports/bifrost-http/lib/config_test.go
- core/schemas/bifrost.go
- helm-charts/bifrost/values-examples/providers-and-virtual-keys.yaml
- helm-charts/bifrost/templates/_helpers.tpl
- transports/bifrost-http/integrations/router.go
- tests/integrations/python/config.json
- transports/bifrost-http/integrations/openai.go
- transports/bifrost-http/handlers/webrtc_realtime.go
- transports/bifrost-http/handlers/webrtc_realtime_test.go
- framework/configstore/rdb.go
- docs/deployment-guides/helm.mdx
- framework/configstore/tables/clientconfig.go
- docs/deployment-guides/config-json/client.mdx
- transports/bifrost-http/lib/config.go
- transports/bifrost-http/handlers/config.go
- framework/configstore/clientconfig.go
- docs/features/governance/virtual-keys.mdx
🚧 Files skipped from review as they are similar to previous changes (1)
- examples/dockers/data/config.json
Pratham-Mishra04
force-pushed
the
05-05-refactor_remove_support_for_direct_api_keys
branch
from
2525d89 to
d45fb1b
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
core/bifrost.go (1)
5722-5803:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate managed-key pinning into the non-rotating branches.
This fixed-key branch only runs after
selectKeyFromProviderForModelWithPool, butListModelsRequestand the multi-key batch/file/container/cached-content paths above bypass that selector and callgetAllSupportedKeys/getKeysForBatchAndFileOpsdirectly. After the direct-key removal,BifrostContextKeyAPIKeyID/BifrostContextKeyAPIKeyNametherefore still fan those request types out across every eligible key, so list-models/key-status results no longer reflect the managed key the caller requested. Please apply the same pinning semantics in those helpers or route these request types through a shared selector first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/bifrost.go` around lines 5722 - 5803, The ListModelsRequest and multi-key batch/file/container/cached-content code paths bypass selectKeyFromProviderForModelWithPool so context pinning (BifrostContextKeyAPIKeyID/BifrostContextKeyAPIKeyName) isn't honored; fix by propagating managed-key pinning into those paths—either update getAllSupportedKeys and getKeysForBatchAndFileOps to accept and respect the context pin (and return keys filtered/ordered to honor the pinned key) or route ListModelsRequest and the multi-key operations through selectKeyFromProviderForModelWithPool so the same pinning/canRotate logic is applied; locate references to getAllSupportedKeys, getKeysForBatchAndFileOps and selectKeyFromProviderForModelWithPool and ensure the pinned key from context is used to filter/position keys before returning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@core/bifrost.go`:
- Around line 5722-5803: The ListModelsRequest and multi-key
batch/file/container/cached-content code paths bypass
selectKeyFromProviderForModelWithPool so context pinning
(BifrostContextKeyAPIKeyID/BifrostContextKeyAPIKeyName) isn't honored; fix by
propagating managed-key pinning into those paths—either update
getAllSupportedKeys and getKeysForBatchAndFileOps to accept and respect the
context pin (and return keys filtered/ordered to honor the pinned key) or route
ListModelsRequest and the multi-key operations through
selectKeyFromProviderForModelWithPool so the same pinning/canRotate logic is
applied; locate references to getAllSupportedKeys, getKeysForBatchAndFileOps and
selectKeyFromProviderForModelWithPool and ensure the pinned key from context is
used to filter/position keys before returning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ade7793a-8cfb-4e64-a130-b870a5dc2c99
📒 Files selected for processing (66)
.github/workflows/configs/withpostgresmcpclientsinconfig/config.jsoncore/bifrost.gocore/internal/llmtests/account.gocore/internal/llmtests/list_models.gocore/providers/utils/utils.gocore/schemas/bifrost.gocore/schemas/context.godocs/deployment-guides/config-json.mdxdocs/deployment-guides/config-json/client.mdxdocs/deployment-guides/config-json/schema-reference.mdxdocs/deployment-guides/helm.mdxdocs/deployment-guides/helm/client.mdxdocs/deployment-guides/helm/values.mdxdocs/enterprise/migration-guides/v1.4.0.mdxdocs/features/governance/virtual-keys.mdxdocs/features/keys-management.mdxdocs/features/observability/default.mdxdocs/integrations/anthropic-sdk/overview.mdxdocs/integrations/bedrock-sdk/overview.mdxdocs/integrations/genai-sdk/overview.mdxdocs/integrations/langchain-sdk.mdxdocs/integrations/litellm-sdk.mdxdocs/integrations/openai-sdk/overview.mdxdocs/integrations/pydanticai-sdk.mdxdocs/migration-guides/v1.5.0.mdxdocs/openapi/openapi.jsondocs/openapi/schemas/management/config.yamldocs/providers/request-options.mdxdocs/quickstart/go-sdk/context-keys.mdxexamples/configs/withpostgresmcpclientsinconfig/config.jsonexamples/configs/withprompushgateway/config.jsonexamples/configs/withvirtualkeys/config.jsonexamples/dockers/data/config.jsonexamples/k8s/examples/values-client-configs.yamlframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/rdb.goframework/configstore/rdb_test.goframework/configstore/tables/clientconfig.gohelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values-examples/providers-and-virtual-keys.yamlhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamlrecipes/values-local-k8s.yamltests/governance/config.jsontests/integrations/python/config.jsontests/integrations/typescript/config.jsontransports/bifrost-http/handlers/config.gotransports/bifrost-http/handlers/webrtc_realtime.gotransports/bifrost-http/handlers/webrtc_realtime_test.gotransports/bifrost-http/handlers/wsresponses.gotransports/bifrost-http/handlers/wsresponses_test.gotransports/bifrost-http/integrations/bedrock.gotransports/bifrost-http/integrations/bedrock_test.gotransports/bifrost-http/integrations/openai.gotransports/bifrost-http/integrations/router.gotransports/bifrost-http/integrations/router_large_payload_test.gotransports/bifrost-http/integrations/router_test.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/lib/config_test.gotransports/bifrost-http/lib/ctx.gotransports/bifrost-http/lib/ctx_test.gotransports/config.schema.jsonui/app/workspace/config/views/securityView.tsxui/lib/types/config.tsui/lib/types/schemas.ts
💤 Files with no reviewable changes (49)
- tests/integrations/python/config.json
- core/schemas/context.go
- tests/integrations/typescript/config.json
- .github/workflows/configs/withpostgresmcpclientsinconfig/config.json
- docs/deployment-guides/config-json.mdx
- examples/k8s/examples/values-client-configs.yaml
- tests/governance/config.json
- recipes/values-local-k8s.yaml
- core/schemas/bifrost.go
- examples/configs/withprompushgateway/config.json
- examples/configs/withpostgresmcpclientsinconfig/config.json
- helm-charts/bifrost/templates/_helpers.tpl
- transports/bifrost-http/integrations/router.go
- framework/configstore/tables/clientconfig.go
- docs/integrations/genai-sdk/overview.mdx
- transports/bifrost-http/lib/config.go
- transports/bifrost-http/handlers/wsresponses.go
- transports/bifrost-http/handlers/webrtc_realtime.go
- docs/deployment-guides/config-json/client.mdx
- transports/bifrost-http/lib/ctx.go
- docs/providers/request-options.mdx
- helm-charts/bifrost/values.yaml
- docs/openapi/openapi.json
- ui/lib/types/config.ts
- examples/configs/withvirtualkeys/config.json
- docs/deployment-guides/config-json/schema-reference.mdx
- helm-charts/bifrost/values.schema.json
- framework/configstore/rdb.go
- transports/config.schema.json
- docs/deployment-guides/helm/values.mdx
- docs/openapi/schemas/management/config.yaml
- docs/integrations/bedrock-sdk/overview.mdx
- docs/integrations/litellm-sdk.mdx
- docs/integrations/anthropic-sdk/overview.mdx
- ui/lib/types/schemas.ts
- framework/configstore/rdb_test.go
- core/internal/llmtests/account.go
- transports/bifrost-http/handlers/webrtc_realtime_test.go
- docs/integrations/langchain-sdk.mdx
- helm-charts/bifrost/values-examples/providers-and-virtual-keys.yaml
- docs/features/governance/virtual-keys.mdx
- framework/configstore/clientconfig.go
- transports/bifrost-http/lib/config_test.go
- transports/bifrost-http/integrations/openai.go
- docs/quickstart/go-sdk/context-keys.mdx
- docs/integrations/openai-sdk/overview.mdx
- docs/deployment-guides/helm.mdx
- docs/integrations/pydanticai-sdk.mdx
- transports/bifrost-http/handlers/config.go
✅ Files skipped from review due to trivial changes (1)
- docs/features/keys-management.mdx
🚧 Files skipped from review as they are similar to previous changes (3)
- transports/bifrost-http/handlers/wsresponses_test.go
- transports/bifrost-http/integrations/router_test.go
- transports/bifrost-http/integrations/bedrock_test.go
Contributor
Merge activity
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR removes the "Direct Key Bypass" feature entirely from both the HTTP gateway and the Go SDK. Previously, callers could pass raw provider API keys via
Authorization,x-api-key,x-goog-api-key,x-bf-bedrock-*, andx-bf-azure-endpointheaders (controlled byallow_direct_keysin config), or inject a key directly into the Go SDK request context viaschemas.BifrostContextKeyDirectKey. All of these paths bypassed Bifrost's key management, governance, rotation, and observability. They have been removed in v1.5.0 and all requests must now resolve to a Bifrost-managed provider key.Changes
schemas.BifrostContextKeyDirectKeycontext key constant and all code branches that checked for it inselectKeyFromProviderForModelWithPool,getAllSupportedKeys, andgetKeysForBatchAndFileOps.allow_direct_keysfromClientConfig, the database table (config_client), the Helm values schema, the JSON config schema, and the OpenAPI spec. A newmigrationDropAllowDirectKeysColumnmigration drops the column on first startup.ShouldAllowDirectKeys()from theHandlerStoreinterface and all implementations.ConvertToBifrostContextthat turnedAuthorization/x-api-key/x-goog-api-keyheader values into direct keys.x-bf-bedrock-*header extraction inbedrockPreCallbackandbedrockBatchPreCallback.AzureEndpointPreHookthat usedx-bf-azure-endpoint+Authorizationto construct a key.createHandlerandhandlePassthrough.BifrostContextKeyDirectKeyclearing/copying steps in the WebRTC realtime handler.ReplicateDirectKeyForListModels) with a key-name pin viaBifrostContextKeyAPIKeyName, removing the last internal use of the direct-key path.allow_direct_keysfield from the TypeScript config types and Zod schema.Type of change
Affected areas
How to test
Verify that:
Authorization: Bearer sk-<raw-provider-key>(non-sk-bf-*) are no longer forwarded as direct keys and instead fall through to normal key selection or return an auth error.allow_direct_keysinconfig.jsonis silently ignored on startup.allow_direct_keyscolumn is dropped fromconfig_clienton first startup of v1.5.0.schemas.BifrostContextKeyDirectKeyfails to compile.BifrostContextKeyAPIKeyNameorBifrostContextKeyAPIKeyIDcontinues to work as the supported alternative.Breaking changes
allow_direct_keysis removed fromconfig.json, the REST API config payload, and the database. HTTP callers that passed raw provider keys viaAuthorization,x-api-key,x-goog-api-key,x-bf-bedrock-*, orx-bf-azure-endpointheaders will no longer have those keys forwarded upstream. Go SDK callers referencingschemas.BifrostContextKeyDirectKeywill not compile. Migrate to Bifrost-managed provider keys and useBifrostContextKeyAPIKeyID/BifrostContextKeyAPIKeyNamefor per-request key pinning, or virtual keys (sk-bf-*) on the HTTP gateway. See Breaking Change 14 in the v1.5.0 migration guide for full details.Security considerations
The direct-key path allowed raw provider credentials to flow through Bifrost without governance controls, bypassing virtual key budgets, rate limits, provider/model allow-lists, and routing rules. It also created a credential leak risk through logs and proxy chains. Removing this path eliminates that surface area and ensures all traffic is subject to Bifrost's key management and observability pipeline.
Checklist
docs/contributing/README.mdand followed the guidelines