refactor: standardize empty array conventions for allowed models#2113
Merged
Pratham-Mishra04 merged 1 commit intov1.5.0from Mar 18, 2026
Merged
Conversation
This was referenced Mar 16, 2026
|
|
12 tasks
Collaborator
Author
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (34)
📝 WalkthroughWalkthroughEmpty arrays now deny all (deny-by-default); Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
![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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/components/ui/modelMultiselect.tsx (1)
83-89:⚠️ Potential issue | 🟡 MinorSingle-select doesn’t normalize
"*"to the friendly label.Multi-select maps wildcard to
ALL_MODELS_OPTION, but single-select still renders"*"directly, causing inconsistent UI text.🛠️ Proposed fix
const selectedOptions: ModelOption[] = isSingleSelect ? stringValue - ? [{ label: stringValue, value: stringValue }] + ? [{ label: stringValue === "*" ? ALL_MODELS_OPTION.label : stringValue, value: stringValue }] : []🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/ui/modelMultiselect.tsx` around lines 83 - 89, selectedOptions currently maps the wildcard "*" to ALL_MODELS_OPTION only for the multi-select branch, leaving the single-select branch to render stringValue raw; update the single-select branch in the selectedOptions calculation to normalize stringValue by checking if stringValue === "*" and, if so, return ALL_MODELS_OPTION (otherwise keep the existing { label: stringValue, value: stringValue }) so both isSingleSelect and multi-select use the same friendly label; reference the selectedOptions variable and the isSingleSelect/stringValue branch when making the change.framework/modelcatalog/main.go (1)
498-500:⚠️ Potential issue | 🟡 MinorFunction docs are stale after the semantic change.
The comment still says empty
allowedModelsuses catalog-based allowance, but Line 534–536 now denies by default. Please update this block to prevent misconfiguration.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/modelcatalog/main.go` around lines 498 - 500, Update the stale function comment to reflect the new semantics: replace the statement that "If allowedModels is empty: Uses model catalog to check if provider supports the model (delegates to GetProvidersForModel...)" with the correct behavior (empty allowedModels now results in deny-by-default), and keep the existing note that a non-empty allowedModels list is checked for matches; reference the allowedModels variable and GetProvidersForModel to locate the comment and ensure the docblock accurately describes the current deny-by-default behavior.
🧹 Nitpick comments (2)
framework/configstore/migrations.go (1)
4948-4953: Consider scoping VK hash recomputation to only modified rows.The current query retrieves all distinct VK IDs from the provider configs table, not just those whose
allowed_modelswas actually updated. For deployments with many VKs, this could result in unnecessary hash recomputations.Since this is a one-time migration, the impact is limited to migration execution time. If performance is acceptable, this can be left as-is.
♻️ Optional optimization to limit scope
- // Recompute config_hash for all VKs that have provider configs - // (any of them may have had their allowed_models updated above). - var modifiedVKIDs []string - if err := tx.Model(&tables.TableVirtualKeyProviderConfig{}). - Distinct("virtual_key_id"). - Pluck("virtual_key_id", &modifiedVKIDs).Error; err != nil { - return fmt.Errorf("failed to query VK IDs for hash recomputation: %w", err) - } + // Recompute config_hash only for VKs whose provider configs were updated. + // These are the ones that now have allowed_models = '["*"]' (i.e., were backfilled). + var modifiedVKIDs []string + if err := tx.Model(&tables.TableVirtualKeyProviderConfig{}). + Where("allowed_models = ?", `["*"]`). + Distinct("virtual_key_id"). + Pluck("virtual_key_id", &modifiedVKIDs).Error; err != nil { + return fmt.Errorf("failed to query VK IDs for hash recomputation: %w", err) + }Note: This assumes all
["*"]values came from this migration. If there are pre-existing["*"]values that weren't changed, this would still process them, but that's better than processing all VKs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/configstore/migrations.go` around lines 4948 - 4953, The migration currently selects all distinct virtual_key_id values from tables.TableVirtualKeyProviderConfig which forces hash recomputation for every VK; restrict the selection to only rows whose allowed_models were set to ["*"] (or otherwise match the specific marker this migration writes) by adding a WHERE predicate (e.g., Where("allowed_models = ?", `["*"]`) or equivalent) to the tx.Model(...).Distinct(...).Pluck(...) query so modifiedVKIDs only contains VKs actually changed by this migration (reference tables.TableVirtualKeyProviderConfig and the modifiedVKIDs variable).ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)
350-352: Consider extracting weight normalization for readability.The inline ternary chain is dense and the formatting places
budget:on the same line as the weight logic, making it harder to follow. Consider extracting to a helper function.♻️ Suggested refactor
+const normalizeWeight = (weight: string | number | undefined | null): number | null => { + if (weight === undefined || weight === null) return null; + if (typeof weight === "string") { + const parsed = parseFloat(weight); + return Number.isNaN(parsed) ? null : parsed; + } + return weight; +}; + const normalizeProviderConfigs = ( configs: NonNullable<FormData["providerConfigs"]>, existingConfigs?: VirtualKey["provider_configs"], ): any[] => { return configs.map((config) => ({ ...config, - weight: config.weight === undefined || config.weight === null - ? null - : typeof config.weight === "string" ? (Number.isNaN(parseFloat(config.weight)) ? null : parseFloat(config.weight)) : config.weight, budget: (() => { + weight: normalizeWeight(config.weight), + budget: (() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 350 - 352, Extract the dense inline ternary that normalizes config.weight into a small helper (e.g., normalizeWeight(value): number | null) and call it where the object currently builds weight in virtualKeySheet (replace the existing inline expression using config.weight). The helper should return null for undefined/null, parse strings with parseFloat and return null for NaN, and return numeric inputs unchanged; keep the budget property on its own line afterwards to improve readability.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@framework/configstore/migrations.go`:
- Around line 4982-4988: The migration updates models_json on TableKey rows but
fails to recompute the associated config_hash, leaving stale hashes; after
performing the Update("models_json", `["*"]`) against
tx.Model(&tables.TableKey{}), load the affected TableKey records (use the same
query predicate), populate their Key.Models from models_json and recompute and
persist config_hash using the same logic as migrationAddConfigHashColumn (the
code path that derives the hash from key fields including Models) so each
updated key has its config_hash recalculated and saved within the same
transaction.
In `@helm-charts/bifrost/values.schema.json`:
- Line 2738: The JSON schema contains unescaped double quotes inside a string
literal in the "description" fields (currently showing ["*"]) which breaks
parsing; update both occurrences of that description to escape the inner quotes
(replace ["*"] with [\"*\"]) so the string is valid JSON—search for the
"description" entries containing ["*"] and change them to use escaped inner
quotes, ensure the file parses after the change.
In `@transports/config.schema.json`:
- Line 1521: The "keys" schema description is misleading because it implies an
array of strings (["*"]) but "keys" is actually an array of objects with a
key_id property; update the "description" for the keys property to state that
each entry is an object with a "key_id" field and give a valid example such as
[{"key_id":"*"}] to show the wildcard usage and note that an empty array denies
all. Reference the "keys" property and the "key_id" field in the description so
readers understand the required JSON shape.
In `@ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx`:
- Around line 209-232: The ModelMultiselect instance in the API keys form needs
a test selector: update the call in apiKeysFormFragment.tsx to pass a
data-testid (e.g. data-testid="api-keys-models-multiselect-trigger"), and if
ModelMultiselect does not accept/forward that prop, modify
ui/components/ui/modelMultiselect.tsx to add a data-testid (or dataTestId) prop
to the component props and forward it to the interactive trigger/input element
used to open/select models; ensure the attribute name follows the pattern
data-testid="<entity>-<element>-<qualifier>" so tests can reliably target the
control.
In `@ui/lib/types/schemas.ts`:
- Line 173: The schema field models currently calls .default(["*"]).optional(),
which prevents the default from applying; change the chain order so .optional()
comes before .default() on the models schema (i.e., update the
z.array(z.string()) chain for models to call .optional() then .default(["*"]))
so the default value is applied when input is undefined.
---
Outside diff comments:
In `@framework/modelcatalog/main.go`:
- Around line 498-500: Update the stale function comment to reflect the new
semantics: replace the statement that "If allowedModels is empty: Uses model
catalog to check if provider supports the model (delegates to
GetProvidersForModel...)" with the correct behavior (empty allowedModels now
results in deny-by-default), and keep the existing note that a non-empty
allowedModels list is checked for matches; reference the allowedModels variable
and GetProvidersForModel to locate the comment and ensure the docblock
accurately describes the current deny-by-default behavior.
In `@ui/components/ui/modelMultiselect.tsx`:
- Around line 83-89: selectedOptions currently maps the wildcard "*" to
ALL_MODELS_OPTION only for the multi-select branch, leaving the single-select
branch to render stringValue raw; update the single-select branch in the
selectedOptions calculation to normalize stringValue by checking if stringValue
=== "*" and, if so, return ALL_MODELS_OPTION (otherwise keep the existing {
label: stringValue, value: stringValue }) so both isSingleSelect and
multi-select use the same friendly label; reference the selectedOptions variable
and the isSingleSelect/stringValue branch when making the change.
---
Nitpick comments:
In `@framework/configstore/migrations.go`:
- Around line 4948-4953: The migration currently selects all distinct
virtual_key_id values from tables.TableVirtualKeyProviderConfig which forces
hash recomputation for every VK; restrict the selection to only rows whose
allowed_models were set to ["*"] (or otherwise match the specific marker this
migration writes) by adding a WHERE predicate (e.g., Where("allowed_models = ?",
`["*"]`) or equivalent) to the tx.Model(...).Distinct(...).Pluck(...) query so
modifiedVKIDs only contains VKs actually changed by this migration (reference
tables.TableVirtualKeyProviderConfig and the modifiedVKIDs variable).
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 350-352: Extract the dense inline ternary that normalizes
config.weight into a small helper (e.g., normalizeWeight(value): number | null)
and call it where the object currently builds weight in virtualKeySheet (replace
the existing inline expression using config.weight). The helper should return
null for undefined/null, parse strings with parseFloat and return null for NaN,
and return numeric inputs unchanged; keep the budget property on its own line
afterwards to improve readability.
🪄 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: aa931ab3-348b-46c2-8811-27c6ecb655f3
📒 Files selected for processing (24)
core/bifrost.gocore/changelog.mdcore/go.modcore/schemas/transcriptions.goframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
💤 Files with no reviewable changes (1)
- core/go.mod
Pratham-Mishra04
changed the title
Pratham-Mishra04
changed the title
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
ef1682b to
366bc98
Compare
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
4e37675 to
0da4bed
Compare
This was referenced Mar 17, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
transports/bifrost-http/handlers/providers.go (1)
753-775:⚠️ Potential issue | 🟠 MajorEmpty-model keys are still treated as allow-all in this path.
With the new semantics, Line 773 returning all models on
!hasRestrictedKeyis incorrect when matched keys exist but all haveModels=[](deny-all). That currently re-opens access in the filtered list API.🛠️ Proposed fix
func (h *ProviderHandler) filterModelsByKeys(provider schemas.ModelProvider, models []string, keyIDs []string) []string { @@ allowedModels := make(map[string]bool) hasRestrictedKey := false hasUnrestrictedKey := false + hasMatchedKey := false for _, keyID := range keyIDs { for _, key := range config.Keys { if key.ID == keyID { + hasMatchedKey = true if slices.Contains(key.Models, "*") { // Key allows all models (wildcard) hasUnrestrictedKey = true } else if len(key.Models) > 0 { @@ if hasUnrestrictedKey { return models } - // If no keys have model restrictions (e.g., unknown key IDs), return all models + // If keys matched but none contributed allowed models, deny all (empty-model semantics). + if hasMatchedKey && !hasRestrictedKey { + return []string{} + } + // If no keys matched, keep current permissive behavior. if !hasRestrictedKey { return models }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/handlers/providers.go` around lines 753 - 775, The code incorrectly treats the case where matched API keys exist but all have Models=[] (deny-all) as "no restrictions" because hasRestrictedKey stays false; add a new flag (e.g., matchedKeyExists or keysMatchedCount) when iterating keys in the same block where hasUnrestrictedKey/hasRestrictedKey are set, and change the final logic so that: if hasUnrestrictedKey return models; else if !hasRestrictedKey and matchedKeyExists return an empty model list (deny all); else if !hasRestrictedKey and !matchedKeyExists return models (no matching keys -> allow all); otherwise build allowedModels as before and return that filtered list using allowedModels and models variables.
🧹 Nitpick comments (1)
ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx (1)
214-224: Centralize["*"]normalization inModelMultiselect.This form is now carrying the allow-all state machine itself. Since the wildcard semantics are being standardized across the stack, consider having
ModelMultiselectemit either["*"]or a star-free list wheneverallowAllOptionis enabled so other callers can’t drift on the same edge cases.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx` around lines 214 - 224, The form currently duplicates wildcard normalization logic inside the onChange handler; move this responsibility into ModelMultiselect so callers receive either ["*"] or a star-free list when allowAllOption is enabled. Update ModelMultiselect's change emitter to apply the rules (emit ["*"] if user selected allow-all, remove "*" if other selections made, or emit selected models as-is) and then simplify this component’s onChange to directly call field.onChange(models) (or whatever value ModelMultiselect emits) without duplicating the star-handling logic; ensure the symbols referenced are ModelMultiselect (producer) and the onChange callback here that calls field.onChange.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@framework/modelcatalog/main.go`:
- Around line 528-536: Update the function/docblock comment above this branch to
reflect the new behavior: when allowedModels is empty the function now denies
all instead of falling back to catalog lookup; mention that
slices.Contains(allowedModels, "*") still allows all and uses
mc.GetProvidersForModel(model) and slices.Contains(supportedProviders, provider)
to decide support, and explicitly document that an empty allowedModels returns
false for any provider/model pair (deny-by-default) so callers of this function
(the code using allowedModels, model, provider, mc.GetProvidersForModel) are not
misled by the old examples.
In `@transports/config.schema.json`:
- Around line 1519-1522: The "keys" array schema currently requires each item to
have "name" and "value", which prevents the documented wildcard sentinel
{"key_id":"*"}; update the "keys"."items" schema to allow a oneOf (or anyOf)
with two alternatives: (1) the wildcard object that requires only "key_id"
(string, pattern "^\\*$" or enum ["*"]) and (2) the existing concrete key object
that requires "key_id", "name" and "value" (and keeps its current constraints).
Ensure the concrete object schema remains unchanged and keep the array-level
description; modify the JSON Schema under the "keys" -> "items" node to
implement this split so the sentinel is accepted.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 755-756: The description fallback incorrectly shows "All models"
when key.models is an empty array (which means deny-all); update the places that
set description (the object with description: key.models?.filter((m) => m !==
"*").join(", ") || "All models") to compute the filtered list into a variable
and, if that filtered array is empty, use a clear deny-all label such as "No
models (deny all)" instead of "All models"; apply the same change to the second
occurrence around the other description/provider assignment so both use
key.models and the new deny-all fallback.
---
Outside diff comments:
In `@transports/bifrost-http/handlers/providers.go`:
- Around line 753-775: The code incorrectly treats the case where matched API
keys exist but all have Models=[] (deny-all) as "no restrictions" because
hasRestrictedKey stays false; add a new flag (e.g., matchedKeyExists or
keysMatchedCount) when iterating keys in the same block where
hasUnrestrictedKey/hasRestrictedKey are set, and change the final logic so that:
if hasUnrestrictedKey return models; else if !hasRestrictedKey and
matchedKeyExists return an empty model list (deny all); else if
!hasRestrictedKey and !matchedKeyExists return models (no matching keys -> allow
all); otherwise build allowedModels as before and return that filtered list
using allowedModels and models variables.
---
Nitpick comments:
In `@ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx`:
- Around line 214-224: The form currently duplicates wildcard normalization
logic inside the onChange handler; move this responsibility into
ModelMultiselect so callers receive either ["*"] or a star-free list when
allowAllOption is enabled. Update ModelMultiselect's change emitter to apply the
rules (emit ["*"] if user selected allow-all, remove "*" if other selections
made, or emit selected models as-is) and then simplify this component’s onChange
to directly call field.onChange(models) (or whatever value ModelMultiselect
emits) without duplicating the star-handling logic; ensure the symbols
referenced are ModelMultiselect (producer) and the onChange callback here that
calls field.onChange.
🪄 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: 4f1ba152-c60d-407f-9ef0-e4435ac38853
📒 Files selected for processing (24)
core/bifrost.gocore/changelog.mdcore/schemas/transcriptions.goframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/asyncMultiselect.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
✅ Files skipped from review due to trivial changes (2)
- transports/bifrost-http/handlers/governance.go
- framework/configstore/tables/virtualkey.go
🚧 Files skipped from review as they are similar to previous changes (11)
- core/bifrost.go
- transports/bifrost-http/server/server.go
- framework/changelog.md
- framework/configstore/tables/key.go
- core/changelog.md
- helm-charts/bifrost/values.schema.json
- core/schemas/transcriptions.go
- framework/configstore/migrations.go
- plugins/governance/resolver.go
- ui/app/workspace/providers/views/providerKeyForm.tsx
- ui/components/ui/modelMultiselect.tsx
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
366bc98 to
5463080
Compare
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
0da4bed to
13d4479
Compare
12 tasks
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 (2)
examples/configs/withvirtualkeys/config.json (1)
207-236:⚠️ Potential issue | 🟡 Minor
bedrock.keys[*].models: []now denies all models in this example.Because this PR standardizes empty arrays as deny-all, these bedrock keys become unusable unless you explicitly set
["*"](or list specific models). That will make the example behave as intended.Suggested fix
- "models": [], + "models": ["*"], ... - "models": [], + "models": ["*"], ... - "models": [], + "models": ["*"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/configs/withvirtualkeys/config.json` around lines 207 - 236, The example config's bedrock key entries currently set "models": [] which now means deny-all; update each affected key object (e.g., id "key-bedrock-us-east-1-prod", "key-bedrock-us-west-2-prod", "key-bedrock-eu-central-1-prod") to use "models": ["*"] (or enumerate allowed model names) so the keys are usable as intended; locate and change the models array inside each bedrock_key entry in config.json accordingly.framework/configstore/tables/virtualkey.go (1)
52-72:⚠️ Potential issue | 🔴 CriticalFix
UnmarshalJSONto populate the correctTableKey.Namefield instead ofKeyID.Line 71 stores config-file key names in the
KeyIDfield, but the resolution logic inCreateVirtualKeyProviderConfig(rdb.go:1832–1843) looks up keys by theNamefield. This mismatch causes key resolution to fail withErrUnresolvedKeyswhenever config-filekey_idsare provided.Config specifies:
"key_ids": ["my-key"]→ stored asTableKey{KeyID: "my-key"}→ resolution looks forName(empty) → fails.Change line 71 to:
pc.Keys[i] = TableKey{Name: keyID}so the key name is stored in the correct field for downstream lookup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@framework/configstore/tables/virtualkey.go` around lines 52 - 72, The UnmarshalJSON logic in TempProviderConfig currently fills pc.Keys with TableKey{KeyID: keyID}, but downstream CreateVirtualKeyProviderConfig resolves keys by TableKey.Name; change the population to use TableKey{Name: keyID} so config key names from temp.KeyIDs are stored in the Name field and will be resolved correctly (keep the rest of the UnmarshalJSON behavior and the existing TempProviderConfig -> TableVirtualKeyProviderConfig assignment).
🧹 Nitpick comments (2)
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (2)
350-352: Simplify weight normalization before object construction.This inline ternary is hard to maintain and easy to break when editing adjacent payload fields. Consider extracting
normalizedWeightfirst.Refactor sketch
- return configs.map((config) => ({ - ...config, - weight: config.weight === undefined || config.weight === null - ? null - : typeof config.weight === "string" ? (Number.isNaN(parseFloat(config.weight)) ? null : parseFloat(config.weight)) : config.weight, budget: (() => { + return configs.map((config) => { + const normalizedWeight = + config.weight == null + ? null + : typeof config.weight === "string" + ? (Number.isNaN(parseFloat(config.weight)) ? null : parseFloat(config.weight)) + : config.weight; + return { + ...config, + weight: normalizedWeight, + budget: (() => { const budgetMaxLimit = normalizeNumericField(config.budget?.max_limit); if (budgetMaxLimit !== undefined) { return { max_limit: budgetMaxLimit, reset_duration: config.budget?.reset_duration || "1M", }; } @@ - })(), - rate_limit: (() => { + })(), + rate_limit: (() => { ... - })(), - })); + })(), + }; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 350 - 352, Extract weight normalization into a separate variable before constructing the payload object: compute a normalizedWeight (e.g., handle undefined/null, parse strings with parseFloat and treat NaN as null, and leave numeric values as-is) and then use normalizedWeight in place of the complex inline ternary currently setting weight in the object (referencing config.weight and the weight field in the object constructed in virtualKeySheet.tsx). This moves the logic out of the inline ternary, makes the payload construction clearer and easier to maintain, and prevents accidental breakage when editing neighboring fields.
755-758: Extract duplicated key model-description logic.The same fallback logic appears twice; consolidating it into a helper will reduce drift risk as semantics evolve.
Refactor sketch
+const formatAllowedModelsDescription = (models?: string[]) => + models == null || models.includes("*") + ? "All models" + : models.filter((m) => m !== "*").join(", ") || "No models (deny all)"; ... - description: - key.models == null || key.models.includes("*") - ? "All models" - : key.models.filter((m) => m !== "*").join(", ") || "No models (deny all)", + description: formatAllowedModelsDescription(key.models), ... - description: - key.models == null || key.models.includes("*") - ? "All models" - : key.models.filter((m) => m !== "*").join(", ") || "No models (deny all)", + description: formatAllowedModelsDescription(key.models),Also applies to: 769-773
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 755 - 758, The model-description expression for keys is duplicated; create a small helper (e.g., function formatKeyModels(models: string[] | null): string or getKeyModelsDescription(key) ) that implements the logic (null or includes("*") => "All models", otherwise filter out "*" and join with ", " or return "No models (deny all)"), then replace both inline expressions in VirtualKeySheet/virtualKeySheet.tsx (where description: key.models == null || key.models.includes("*") ? ... and the similar block later) with calls to that helper so the logic is centralized and reused.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/features/governance/routing.mdx`:
- Around line 293-295: Update this page's narrative and examples to reflect the
new semantics for key_id lists: empty arrays ([]) are deny-all and ["*"] is
explicit allow-all. Locate references and examples that mention "key_ids" and
any text saying empty arrays are permissive and change them to state deny-all;
replace permissive examples that used [] with ["*"] and adjust explanatory copy
to show the correct behavior. Also update any sample JSON/yaml blocks that show
"key_ids": [] to instead demonstrate deny-all or show "key_ids": ["*"] for
allow-all so examples and prose are consistent.
---
Outside diff comments:
In `@examples/configs/withvirtualkeys/config.json`:
- Around line 207-236: The example config's bedrock key entries currently set
"models": [] which now means deny-all; update each affected key object (e.g., id
"key-bedrock-us-east-1-prod", "key-bedrock-us-west-2-prod",
"key-bedrock-eu-central-1-prod") to use "models": ["*"] (or enumerate allowed
model names) so the keys are usable as intended; locate and change the models
array inside each bedrock_key entry in config.json accordingly.
In `@framework/configstore/tables/virtualkey.go`:
- Around line 52-72: The UnmarshalJSON logic in TempProviderConfig currently
fills pc.Keys with TableKey{KeyID: keyID}, but downstream
CreateVirtualKeyProviderConfig resolves keys by TableKey.Name; change the
population to use TableKey{Name: keyID} so config key names from temp.KeyIDs are
stored in the Name field and will be resolved correctly (keep the rest of the
UnmarshalJSON behavior and the existing TempProviderConfig ->
TableVirtualKeyProviderConfig assignment).
---
Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 350-352: Extract weight normalization into a separate variable
before constructing the payload object: compute a normalizedWeight (e.g., handle
undefined/null, parse strings with parseFloat and treat NaN as null, and leave
numeric values as-is) and then use normalizedWeight in place of the complex
inline ternary currently setting weight in the object (referencing config.weight
and the weight field in the object constructed in virtualKeySheet.tsx). This
moves the logic out of the inline ternary, makes the payload construction
clearer and easier to maintain, and prevents accidental breakage when editing
neighboring fields.
- Around line 755-758: The model-description expression for keys is duplicated;
create a small helper (e.g., function formatKeyModels(models: string[] | null):
string or getKeyModelsDescription(key) ) that implements the logic (null or
includes("*") => "All models", otherwise filter out "*" and join with ", " or
return "No models (deny all)"), then replace both inline expressions in
VirtualKeySheet/virtualKeySheet.tsx (where description: key.models == null ||
key.models.includes("*") ? ... and the similar block later) with calls to that
helper so the logic is centralized and reused.
🪄 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: 2cc1df1f-ec5b-4b2b-9247-b9a8ee6ff605
📒 Files selected for processing (27)
core/bifrost.gocore/changelog.mdcore/schemas/transcriptions.godocs/features/governance/routing.mdxexamples/configs/withvirtualkeys/config.jsonframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/asyncMultiselect.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
💤 Files with no reviewable changes (1)
- transports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (1)
- framework/changelog.md
🚧 Files skipped from review as they are similar to previous changes (11)
- transports/bifrost-http/handlers/providers.go
- transports/bifrost-http/handlers/governance.go
- ui/components/ui/asyncMultiselect.tsx
- plugins/governance/resolver.go
- framework/configstore/tables/key.go
- framework/configstore/migrations.go
- ui/app/workspace/providers/views/providerKeyForm.tsx
- ui/lib/types/schemas.ts
- ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx
- plugins/governance/resolver_test.go
- core/bifrost.go
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
5463080 to
a6efe9a
Compare
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
13d4479 to
2ee4667
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
examples/configs/withvirtualkeys/config.json (1)
207-238:⚠️ Potential issue | 🔴 CriticalBedrock keys need populated
modelsarrays to allow requests.Empty
modelsarrays trigger deny-by-default semantics and will block all requests. According to the schema: "empty array denies all (deny-by-default)". Update each Bedrock key to use"models": ["*"]to match the pattern used by other provider keys (Azure, Vertex, OpenAI), unless specific model filtering is intended.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/configs/withvirtualkeys/config.json` around lines 207 - 238, The Bedrock key entries ("id": "key-bedrock-us-east-1-prod", "key-bedrock-us-west-2-prod", and "key-bedrock-eu-central-1-prod") have empty "models" arrays which trigger deny-by-default; update each of those entries' "models" property to ["*"] (or another explicit allow list if intended) so they permit requests—modify the "models" field in the corresponding objects (identified by their id or name fields) from [] to ["*"].ui/components/ui/modelMultiselect.tsx (1)
85-91:⚠️ Potential issue | 🟡 MinorMap
"*"toALL_MODELS_OPTIONin single-select mode too.In single-select, value
"*"is currently displayed as"*", while multi-select shows “Allow All Models”. Keep both modes consistent.💡 Suggested fix
const selectedOptions: ModelOption[] = isSingleSelect ? stringValue - ? [{ label: stringValue, value: stringValue }] + ? [stringValue === "*" ? ALL_MODELS_OPTION : { label: stringValue, value: stringValue }] : [] : arrayValue.map((model) => ( model === "*" ? ALL_MODELS_OPTION : { label: model, value: model } ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/ui/modelMultiselect.tsx` around lines 85 - 91, The selectedOptions calculation treats "*" specially only for multi-select; update the single-select branch so when isSingleSelect and stringValue === "*" you return ALL_MODELS_OPTION (i.e., map the "*" value to ALL_MODELS_OPTION) instead of the raw string; modify the ternary that builds selectedOptions (references: selectedOptions, isSingleSelect, stringValue, arrayValue, ALL_MODELS_OPTION) so both single- and multi-select use the same mapping logic for "*".
🧹 Nitpick comments (1)
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)
351-353: Run Prettier on the newweightnormalization block.
budgetis currently glued onto the tail of theweightternary, which makes this object literal hard to scan and doesn't match the repo formatting rule.As per coding guidelines, "TypeScript/React code must be formatted with Prettier".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 351 - 353, The object literal's `weight` normalization expression is malformed onto a single line so `budget` is concatenated to its tail; run Prettier (or reformat) the `weight` block so the ternary/conditional expression for `config.weight` is properly multiline and `budget` starts on its own line. Locate the `weight: config.weight === undefined || config.weight === null ? null : typeof config.weight === "string" ? (Number.isNaN(parseFloat(config.weight)) ? null : parseFloat(config.weight)) : config.weight, budget: ...` expression in virtualKeySheet.tsx and reformat it (or run the repo Prettier configuration) to break the nested ternary into readable lines and place `budget:` on the next line to match project formatting rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@framework/configstore/tables/virtualkey.go`:
- Line 52: The comment for the KeyIDs field in virtualkey.go is misleading;
update the comment on KeyIDs []string `json:"key_ids"` to reflect that these are
key identifiers that populate TableKey.KeyID (not arbitrary key names) and
retain the usage note (e.g., use ["*"] to allow all); reference the KeyIDs field
and TableKey.KeyID so readers understand the exact mapping and semantics.
- Around line 63-73: The unmarshal branch leaves pc.AllowAllKeys true when
non-wildcard temp.KeyIDs are supplied, creating an ambiguous state; update the
logic in the function handling temp.KeyIDs (referencing temp.KeyIDs, pc.Keys,
pc.AllowAllKeys) so that when you set concrete pc.Keys you explicitly clear
pc.AllowAllKeys (set it false), and conversely when detecting the wildcard ("*")
set pc.AllowAllKeys true and ensure pc.Keys is empty; make these assignments
together to avoid any contradictory state.
In `@transports/config.schema.json`:
- Around line 1532-1536: The JSON schema for the key_ids array currently allows
empty strings which are invalid as key names/UUIDs; update the items schema for
"key_ids" (the items object under the "key_ids" property) to require non-empty
strings by adding "minLength": 1 to the item definition so each array element
must be at least one character long.
- Around line 1517-1519: Add an explicit default of ["*"] to
virtual_key_provider_config.allowed_models so configs that omit this field
inherit the stack-wide allow-all default (match the existing behavior of
base_key.models); locate the allowed_models property in the
virtual_key_provider_config schema and set its default to ["*"], and mirror the
same default for the other allowed_models occurrence referenced in the schema to
keep consistency with base_key.models.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 706-710: The closure that computes keys for ModelMultiselect
treats config.key_ids = ["*"] as an empty set; update it to detect the wildcard
and expand it to all provider key_ids before filtering: inside the keys
computation (referencing availableKeys, config, providerKeys, and configKeyIds)
check if config.key_ids is exactly ["*"] and in that case set configKeyIds to
providerKeys.map(k => k.key_id) so ModelMultiselect receives all provider keys;
leave existing behavior for other config.key_ids values intact.
---
Outside diff comments:
In `@examples/configs/withvirtualkeys/config.json`:
- Around line 207-238: The Bedrock key entries ("id":
"key-bedrock-us-east-1-prod", "key-bedrock-us-west-2-prod", and
"key-bedrock-eu-central-1-prod") have empty "models" arrays which trigger
deny-by-default; update each of those entries' "models" property to ["*"] (or
another explicit allow list if intended) so they permit requests—modify the
"models" field in the corresponding objects (identified by their id or name
fields) from [] to ["*"].
In `@ui/components/ui/modelMultiselect.tsx`:
- Around line 85-91: The selectedOptions calculation treats "*" specially only
for multi-select; update the single-select branch so when isSingleSelect and
stringValue === "*" you return ALL_MODELS_OPTION (i.e., map the "*" value to
ALL_MODELS_OPTION) instead of the raw string; modify the ternary that builds
selectedOptions (references: selectedOptions, isSingleSelect, stringValue,
arrayValue, ALL_MODELS_OPTION) so both single- and multi-select use the same
mapping logic for "*".
---
Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 351-353: The object literal's `weight` normalization expression is
malformed onto a single line so `budget` is concatenated to its tail; run
Prettier (or reformat) the `weight` block so the ternary/conditional expression
for `config.weight` is properly multiline and `budget` starts on its own line.
Locate the `weight: config.weight === undefined || config.weight === null ? null
: typeof config.weight === "string" ? (Number.isNaN(parseFloat(config.weight)) ?
null : parseFloat(config.weight)) : config.weight, budget: ...` expression in
virtualKeySheet.tsx and reformat it (or run the repo Prettier configuration) to
break the nested ternary into readable lines and place `budget:` on the next
line to match project formatting rules.
🪄 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: dc78b5ae-478e-4c0b-9686-0e445b66c577
📒 Files selected for processing (28)
core/bifrost.gocore/changelog.mdcore/go.modcore/schemas/transcriptions.godocs/features/governance/routing.mdxexamples/configs/withvirtualkeys/config.jsonframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/asyncMultiselect.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
💤 Files with no reviewable changes (2)
- core/go.mod
- transports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (2)
- core/changelog.md
- transports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (12)
- helm-charts/bifrost/values.schema.json
- framework/modelcatalog/main.go
- ui/lib/types/schemas.ts
- ui/components/ui/asyncMultiselect.tsx
- transports/bifrost-http/server/server.go
- transports/bifrost-http/handlers/providers.go
- transports/bifrost-http/handlers/governance.go
- framework/changelog.md
- ui/app/workspace/providers/fragments/apiKeysFormFragment.tsx
- ui/app/workspace/providers/views/providerKeyForm.tsx
- docs/features/governance/routing.mdx
- ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
2ee4667 to
80ebe07
Compare
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
a6efe9a to
0e423ab
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/configs/withvirtualkeys/config.json (1)
207-236:⚠️ Potential issue | 🟠 MajorBedrock example keys now resolve to deny-all due to empty
modelsarrays.Under the new semantics,
[]denies all. If these sample keys are meant to be usable, switch to["*"]explicitly.🔧 Suggested fix
{ "id": "key-bedrock-us-east-1-prod", @@ - "models": [], + "models": ["*"], @@ { "id": "key-bedrock-us-west-2-prod", @@ - "models": [], + "models": ["*"], @@ { "id": "key-bedrock-eu-central-1-prod", @@ - "models": [], + "models": ["*"],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/configs/withvirtualkeys/config.json` around lines 207 - 236, The sample Bedrock key entries (e.g., id "key-bedrock-us-east-1-prod", "key-bedrock-us-west-2-prod", "key-bedrock-eu-central-1-prod") use empty "models" arrays which now resolve to deny-all; update each key's "models" value from [] to ["*"] so these sample keys are usable under the new semantics—search for the objects with those ids and replace their empty "models" arrays with ["*"].
♻️ Duplicate comments (1)
transports/config.schema.json (1)
1532-1538:⚠️ Potential issue | 🟡 MinorReject empty strings in
key_ids.
items: { "type": "string" }still accepts[""], which cannot resolve to either a config key name or an API UUID. AddingminLength: 1keeps this schema useful as a guardrail.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/config.schema.json` around lines 1532 - 1538, The key_ids array currently allows empty strings because items only specify "type": "string"; update the "key_ids" schema's items definition (the "key_ids" property) to reject empty strings by adding "minLength": 1 under items so each element must be a non-empty string.
🧹 Nitpick comments (2)
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)
758-761: Minor cleanup: extract duplicated key-model description formatting.The same fallback expression appears twice; a small helper would keep this consistent and easier to maintain.
🧹 Suggested cleanup
+const getKeyModelsDescription = (models?: string[]) => + models == null || models.includes("*") + ? "All models" + : models.filter((m) => m !== "*").join(", ") || "No models (deny all)"; ... - description: - key.models == null || key.models.includes("*") - ? "All models" - : key.models.filter((m) => m !== "*").join(", ") || "No models (deny all)", + description: getKeyModelsDescription(key.models), ... - description: - key.models == null || key.models.includes("*") - ? "All models" - : key.models.filter((m) => m !== "*").join(", ") || "No models (deny all)", + description: getKeyModelsDescription(key.models),Also applies to: 772-775
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 758 - 761, The duplicated fallback expression for describing key.models in virtualKeySheet.tsx should be extracted into a small helper (e.g., formatKeyModels or describeKeyModels) that accepts the models array (string[] | null) and returns "All models", "No models (deny all)" or a comma-joined list excluding "*" as currently implemented; replace both inline expressions (the description assignments around the key.models checks at the two locations shown) with calls to this helper to keep logic consistent and easier to maintain.ui/components/ui/modelMultiselect.tsx (1)
167-176: Consider normalizing wildcard exclusivity insideModelMultiselect.
"*"handling is now duplicated in consumers. Centralizing it here (whenallowAllOptionis true) would reduce repeated logic and prevent behavior drift.♻️ Suggested refactor
const handleChange = useCallback( (options: Option<ModelOption>[]) => { if (isSingleSelect) { const selected = options[0]; (onChange as (model: string) => void)(selected?.value || ""); } else { - const modelNames = options.map((opt) => opt.value); + const rawModelNames = options.map((opt) => opt.value); + const modelNames = + allowAllOption && rawModelNames.includes("*") + ? rawModelNames.length > 1 + ? rawModelNames.filter((m) => m !== "*") + : ["*"] + : rawModelNames; (onChange as (models: string[]) => void)(modelNames); } @@ - [onChange, provider, keys, getModels, getBaseModels, isSingleSelect, shouldLoadOnEmpty, shouldUseBaseModels], + [onChange, provider, keys, getModels, getBaseModels, isSingleSelect, shouldLoadOnEmpty, shouldUseBaseModels, allowAllOption], );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/ui/modelMultiselect.tsx` around lines 167 - 176, Centralize wildcard ("*") exclusivity in ModelMultiselect by updating the handleChange callback: when allowAllOption is true and the incoming options include the "*" value, ensure "*" is the only selected value (for single-select and multi-select paths), and when "*" is not present remove any "*" from the selection; then call onChange with the normalized value(s). Locate handleChange in ModelMultiselect and adjust the Option<ModelOption>[] handling to filter/normalize "*" before mapping values (preserve current onChange type casts).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/providers/mistral/models.go`:
- Around line 20-21: The filtering logic treats ["*"] as a literal value so real
models get excluded; update the condition in the model filtering blocks (the if
that currently reads: if !unfiltered && len(allowedModels) > 0 &&
!slices.Contains(allowedModels, model.ID) { continue }) to also allow the
wildcard by adding a check for slices.Contains(allowedModels, "*") (e.g. if
!unfiltered && len(allowedModels) > 0 && !slices.Contains(allowedModels,
model.ID) && !slices.Contains(allowedModels, "*") { continue }), and apply the
same change to the second occurrence at the other block (the lines referenced in
the comment).
In `@framework/configstore/tables/virtualkey.go`:
- Around line 63-74: The code currently treats ["*", "id"] as a literal key
list; add a validation before the existing branch on temp.KeyIDs to reject mixed
wildcard and explicit IDs by checking temp.KeyIDs for the presence of "*" and if
found while len(temp.KeyIDs) > 1 return/fail fast with a clear error; keep the
existing handling for the single "*" case that sets pc.AllowAllKeys = true and
for all-other-items populate pc.Keys = make([]TableKey, ...) and assign
TableKey{KeyID: keyID}.
In `@transports/bifrost-http/handlers/providers.go`:
- Around line 754-765: The loop handling matched keys currently treats an empty
key.Models as "contributes nothing", which leaves hasRestrictedKey false and
causes the fallback to return all models; change the logic to treat
len(key.Models)==0 as an explicit deny-all: set a new flag (e.g., hasDenyAllKey
= true) when encountering an empty Models slice and short-circuit to return an
empty models list (or ensure subsequent logic checks hasDenyAllKey before the
fallback), rather than falling through; update the code paths that use
hasUnrestrictedKey, hasRestrictedKey, and allowedModels so they respect
hasDenyAllKey and do not return the full models slice for deny-all keys.
---
Outside diff comments:
In `@examples/configs/withvirtualkeys/config.json`:
- Around line 207-236: The sample Bedrock key entries (e.g., id
"key-bedrock-us-east-1-prod", "key-bedrock-us-west-2-prod",
"key-bedrock-eu-central-1-prod") use empty "models" arrays which now resolve to
deny-all; update each key's "models" value from [] to ["*"] so these sample keys
are usable under the new semantics—search for the objects with those ids and
replace their empty "models" arrays with ["*"].
---
Duplicate comments:
In `@transports/config.schema.json`:
- Around line 1532-1538: The key_ids array currently allows empty strings
because items only specify "type": "string"; update the "key_ids" schema's items
definition (the "key_ids" property) to reject empty strings by adding
"minLength": 1 under items so each element must be a non-empty string.
---
Nitpick comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 758-761: The duplicated fallback expression for describing
key.models in virtualKeySheet.tsx should be extracted into a small helper (e.g.,
formatKeyModels or describeKeyModels) that accepts the models array (string[] |
null) and returns "All models", "No models (deny all)" or a comma-joined list
excluding "*" as currently implemented; replace both inline expressions (the
description assignments around the key.models checks at the two locations shown)
with calls to this helper to keep logic consistent and easier to maintain.
In `@ui/components/ui/modelMultiselect.tsx`:
- Around line 167-176: Centralize wildcard ("*") exclusivity in ModelMultiselect
by updating the handleChange callback: when allowAllOption is true and the
incoming options include the "*" value, ensure "*" is the only selected value
(for single-select and multi-select paths), and when "*" is not present remove
any "*" from the selection; then call onChange with the normalized value(s).
Locate handleChange in ModelMultiselect and adjust the Option<ModelOption>[]
handling to filter/normalize "*" before mapping values (preserve current
onChange type casts).
🪄 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: 241da39f-5609-4fdf-9bc6-7f8b22588703
📒 Files selected for processing (29)
core/bifrost.gocore/changelog.mdcore/providers/mistral/mistral.gocore/providers/mistral/models.gocore/schemas/transcriptions.godocs/features/governance/routing.mdxexamples/configs/withvirtualkeys/config.jsonframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/asyncMultiselect.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
💤 Files with no reviewable changes (1)
- transports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (1)
- transports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (8)
- ui/components/ui/asyncMultiselect.tsx
- transports/bifrost-http/server/server.go
- transports/bifrost-http/handlers/governance.go
- framework/configstore/migrations.go
- helm-charts/bifrost/values.schema.json
- framework/changelog.md
- core/changelog.md
- core/schemas/transcriptions.go
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
80ebe07 to
413b2bb
Compare
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
0e423ab to
102a0a7
Compare
9 tasks
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
413b2bb to
24b4bdb
Compare
Collaborator
Author
|
@coderabbitai full-review |
Contributor
|
✅ Actions performedFull review triggered. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/components/ui/modelMultiselect.tsx (1)
85-91:⚠️ Potential issue | 🟡 MinorHandle wildcard label in single-select mode too.
At Line 87, single-select still renders
"*"as a literal label, while multi-select maps it toALL_MODELS_OPTION. This creates inconsistent UI for the same semantic value.💡 Suggested fix
const selectedOptions: ModelOption[] = isSingleSelect ? stringValue - ? [{ label: stringValue, value: stringValue }] + ? [stringValue === "*" ? ALL_MODELS_OPTION : { label: stringValue, value: stringValue }] : [] : arrayValue.map((model) => ( model === "*" ? ALL_MODELS_OPTION : { label: model, value: model } ));🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/components/ui/modelMultiselect.tsx` around lines 85 - 91, selectedOptions builds the UI option list but treats the wildcard "*" differently between single- and multi-select, causing inconsistent display; update the single-select branch (where isSingleSelect and stringValue are used) to map stringValue === "*" to ALL_MODELS_OPTION (the same mapping used in the arrayValue branch) so both single- and multi-select use ALL_MODELS_OPTION for the wildcard and keep labels consistent.
♻️ Duplicate comments (2)
transports/config.schema.json (1)
1532-1536:⚠️ Potential issue | 🟡 Minor
key_idsstill allows empty strings.Line 1536 accepts
"", which cannot represent a valid key name or UUID. Add a minimum length constraint.🛠️ Suggested fix
"key_ids": { "type": "array", "description": "Keys allowed for this provider config. Use [\"*\"] to allow all keys; empty array denies all (deny-by-default). In config.json, values are key names. Via the API, values are key UUIDs.", "items": { - "type": "string" + "type": "string", + "minLength": 1 } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/config.schema.json` around lines 1532 - 1536, The "key_ids" array currently allows empty strings; update the schema for the "key_ids" property's "items" (the object under "key_ids") to include a string length constraint by adding "minLength": 1 so empty "" values are rejected while preserving "type": "string" for items.core/providers/mistral/models.go (1)
20-21:⚠️ Potential issue | 🔴 CriticalWildcard
["*"]is still treated as a literal model ID.At Line 20 and Line 35, wildcard is not special-cased. For
allowedModels=["*"], real models are skipped andmistral/*gets backfilled, which breaks allow-all semantics.🔧 Suggested fix
includedModels := make(map[string]bool) + allowAll := slices.Contains(allowedModels, "*") for _, model := range response.Data { - if !unfiltered && len(allowedModels) > 0 && !slices.Contains(allowedModels, model.ID) { + if !unfiltered && !allowAll && len(allowedModels) > 0 && !slices.Contains(allowedModels, model.ID) { continue } @@ - if !unfiltered && len(allowedModels) > 0 { + if !unfiltered && !allowAll && len(allowedModels) > 0 { for _, allowedModel := range allowedModels { if !includedModels[allowedModel] {Also applies to: 35-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/mistral/models.go` around lines 20 - 21, The loop currently treats the wildcard "*" as a literal model ID because the membership check uses slices.Contains(allowedModels, model.ID) without special-casing "*"; add a small guard (e.g., compute allowAll := slices.Contains(allowedModels, "*") before the loop) and change the filter condition used around model.ID (the clause that currently reads !unfiltered && len(allowedModels) > 0 && !slices.Contains(allowedModels, model.ID)) to skip the slices.Contains check when allowAll is true (i.e., only apply the allowedModels membership test when allowAll is false); apply the same fix to the analogous condition around line 35 so that ["*"] correctly means allow-all.
🧹 Nitpick comments (3)
transports/bifrost-http/server/server.go (1)
536-539: Consolidate wildcard model filtering into a shared helper.The same
model == "*"filtering is duplicated in three places. Centralizing it will keep wildcard semantics consistent as this stack evolves.♻️ Suggested refactor
+func appendExplicitModels(provider schemas.ModelProvider, keyModels []string, out []schemas.Model) []schemas.Model { + for _, model := range keyModels { + if model == "*" || model == "" { + continue + } + out = append(out, schemas.Model{ID: string(provider) + "/" + model}) + } + return out +}- for _, model := range key.Models { - if model == "*" { - continue - } - modelsInKeys = append(modelsInKeys, schemas.Model{ - ID: string(provider) + "/" + model, - }) - } + modelsInKeys = appendExplicitModels(provider, key.Models, modelsInKeys)Apply similarly to the other two loops in this file.
Also applies to: 771-773, 1263-1265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/server/server.go` around lines 536 - 539, Extract the duplicate wildcard check (model == "*") into a small shared helper (e.g., shouldSkipModel or isWildcardModel) and replace the inline check in each loop that currently does `if model == "*" { continue }` with a call to that helper; update all three occurrences in server.go (the loop blocks referencing the `model` variable around the shown diff and the other two similar loops) so they all use the new helper to decide whether to continue, keeping the helper package-private to this file for consistency.core/bifrost.go (1)
6176-6202: Update the stale model-filter comment to match the new deny-by-default behavior.Line 6176 still says empty
Modelsmeans allow-all, but Line 6201-Line 6202 now enforce emptyModelsas deny-all. Please align the comment to avoid future regressions.✏️ Suggested comment-only diff
- // filter out keys which don't support the model, if the key has no models, it is supported for all models + // Filter keys by model allowlist semantics: + // - ["*"] = allow all models + // - [] = deny all models + // - explicit list = allow only listed models var supportedKeys []schemas.Key🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/bifrost.go` around lines 6176 - 6202, The comment describing model filtering is outdated: it claims an empty key.Models allows all models while the code in the modelSupported computation now treats an empty list as deny-all; update the comment near the skipModelCheck / model filtering logic (around variables supportedKeys, skipModelCheck and the modelSupported calculation) to state that: ["*"] allows all models, an empty slice denies all models, and a specific list allows only listed models; ensure the explanation references hasValue and CanProviderKeyValueBeEmpty(baseProviderType) as prerequisites for modelSupported.ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)
351-353: Refactor weight normalization for readability and safer maintenance.Line 351-Line 353 works, but the nested inline ternary makes this branch hard to audit and easy to regress later.
♻️ Suggested refactor
+const parseWeight = (weight: string | number | null | undefined): number | null => { + if (weight === undefined || weight === null) return null; + if (typeof weight === "number") return weight; + const parsed = Number.parseFloat(weight); + return Number.isNaN(parsed) ? null : parsed; +}; + return configs.map((config) => ({ ...config, - weight: config.weight === undefined || config.weight === null - ? null - : typeof config.weight === "string" ? (Number.isNaN(parseFloat(config.weight)) ? null : parseFloat(config.weight)) : config.weight, budget: (() => { + weight: parseWeight(config.weight), + budget: (() => {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 351 - 353, The inline nested ternary used to compute weight in the virtual key object (the expression referencing config.weight) is hard to read and brittle; refactor by extracting a small helper or local variable (e.g., normalizeWeight or weightNormalized) that explicitly handles undefined/null => null, string => parseFloat with Number.isNaN check returning null on failure, and numeric values returned as-is, then use that symbol in place of the long ternary when building the object (update the code around the weight: ... and budget: (() => { ... ) block in virtualKeySheet.tsx).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@core/changelog.md`:
- Line 3: Update the changelog entry that currently states "standardize empty
array conventions in bifrost. Empty array means no tools/keys are allowed,
[\"*\"] means all tools/keys are allowed." to also document model semantics by
adding that the same convention applies to models (e.g., models: [] denies all,
[\"*\"] allows all), ensuring the entry explicitly mentions tools, keys, and
models so the change is accurately described.
In `@examples/configs/withvirtualkeys/config.json`:
- Around line 88-90: The Bedrock key referenced by "key_ids":
["key-bedrock-eu-central-1-prod"] is currently configured with an empty models
list (models: []), which results in deny-all access; update the key definition
for key-bedrock-eu-central-1-prod to include the allowed model names (or remove
the explicit key reference if you intend to inherit defaults) so model routing
can match, ensuring the models array contains the proper Bedrock model
identifiers instead of being empty.
In `@framework/changelog.md`:
- Line 3: The changelog entry "refactor: standardize empty array conventions in
modelcatalog and tables." must be reclassified as a breaking change and
expanded: update the line in changelog.md to a "breaking:" heading, explicitly
state that empty arrays now mean deny-all and ["*"] means allow-all for
modelcatalog and tables, and add a short migration note advising users to audit
configs and replace empty arrays with ["*"] or explicit entries as needed;
reference the modelcatalog and tables convention change so readers can find
related PRs.
In `@framework/configstore/migrations.go`:
- Around line 4944-4945: The predicate that targets rows with allowed_models set
to an empty JSON array or NULL misses rows where the JSON column contains the
literal text "null"; update the Where predicates around the
Update("allowed_models", `["*"]`) calls to also match the JSON-string literal
`"null"` (e.g. add OR allowed_models = ? with the second arg set to `"null"`),
and apply this same change to both occurrences near the existing
Where(...).Update("allowed_models", `["*"]`) calls so those rows are included in
the backfill.
- Around line 4997-5006: The constructed schemas.Key in the migration omits
fields used by GenerateKeyHash (e.g., VLLMKeyConfig, Enabled, UseForBatchAPI),
causing incorrect config_hash values; update the construction of schemaKey
inside the migration to populate all fields that GenerateKeyHash depends on
(including VLLMKeyConfig, Enabled, UseForBatchAPI and any other key fields),
keeping existing fields like Name, Value, Models, Weight (via getWeight),
AzureKeyConfig, VertexKeyConfig, BedrockKeyConfig, and ReplicateKeyConfig so
GenerateKeyHash produces the correct hash and prevents false config drift.
In `@framework/configstore/tables/key.go`:
- Around line 92-97: When persisting the Key struct, normalize an omitted or
nil/empty k.Models to the wildcard ["*"] before json.Marshal so you don't store
a deny-all; update the code around the block that sets k.ModelsJSON (where
k.Models is marshalled) to check if k.Models == nil || len(k.Models) == 0 and if
so set k.Models = []string{"*"} prior to marshalling, then marshal into
k.ModelsJSON as currently done (ensure this change touches the same method that
contains k.Models, k.ModelsJSON and k.Enabled).
---
Outside diff comments:
In `@ui/components/ui/modelMultiselect.tsx`:
- Around line 85-91: selectedOptions builds the UI option list but treats the
wildcard "*" differently between single- and multi-select, causing inconsistent
display; update the single-select branch (where isSingleSelect and stringValue
are used) to map stringValue === "*" to ALL_MODELS_OPTION (the same mapping used
in the arrayValue branch) so both single- and multi-select use ALL_MODELS_OPTION
for the wildcard and keep labels consistent.
---
Duplicate comments:
In `@core/providers/mistral/models.go`:
- Around line 20-21: The loop currently treats the wildcard "*" as a literal
model ID because the membership check uses slices.Contains(allowedModels,
model.ID) without special-casing "*"; add a small guard (e.g., compute allowAll
:= slices.Contains(allowedModels, "*") before the loop) and change the filter
condition used around model.ID (the clause that currently reads !unfiltered &&
len(allowedModels) > 0 && !slices.Contains(allowedModels, model.ID)) to skip the
slices.Contains check when allowAll is true (i.e., only apply the allowedModels
membership test when allowAll is false); apply the same fix to the analogous
condition around line 35 so that ["*"] correctly means allow-all.
In `@transports/config.schema.json`:
- Around line 1532-1536: The "key_ids" array currently allows empty strings;
update the schema for the "key_ids" property's "items" (the object under
"key_ids") to include a string length constraint by adding "minLength": 1 so
empty "" values are rejected while preserving "type": "string" for items.
---
Nitpick comments:
In `@core/bifrost.go`:
- Around line 6176-6202: The comment describing model filtering is outdated: it
claims an empty key.Models allows all models while the code in the
modelSupported computation now treats an empty list as deny-all; update the
comment near the skipModelCheck / model filtering logic (around variables
supportedKeys, skipModelCheck and the modelSupported calculation) to state that:
["*"] allows all models, an empty slice denies all models, and a specific list
allows only listed models; ensure the explanation references hasValue and
CanProviderKeyValueBeEmpty(baseProviderType) as prerequisites for
modelSupported.
In `@transports/bifrost-http/server/server.go`:
- Around line 536-539: Extract the duplicate wildcard check (model == "*") into
a small shared helper (e.g., shouldSkipModel or isWildcardModel) and replace the
inline check in each loop that currently does `if model == "*" { continue }`
with a call to that helper; update all three occurrences in server.go (the loop
blocks referencing the `model` variable around the shown diff and the other two
similar loops) so they all use the new helper to decide whether to continue,
keeping the helper package-private to this file for consistency.
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 351-353: The inline nested ternary used to compute weight in the
virtual key object (the expression referencing config.weight) is hard to read
and brittle; refactor by extracting a small helper or local variable (e.g.,
normalizeWeight or weightNormalized) that explicitly handles undefined/null =>
null, string => parseFloat with Number.isNaN check returning null on failure,
and numeric values returned as-is, then use that symbol in place of the long
ternary when building the object (update the code around the weight: ... and
budget: (() => { ... ) block in virtualKeySheet.tsx).
🪄 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: ae2d2bfe-edc2-489c-aa00-3a916c5d39fe
📒 Files selected for processing (30)
core/bifrost.gocore/changelog.mdcore/go.modcore/providers/mistral/mistral.gocore/providers/mistral/models.gocore/schemas/transcriptions.godocs/features/governance/routing.mdxexamples/configs/withvirtualkeys/config.jsonframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/asyncMultiselect.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
💤 Files with no reviewable changes (2)
- core/go.mod
- transports/bifrost-http/lib/config.go
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
24b4bdb to
dd9d995
Compare
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
102a0a7 to
033e271
Compare
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
dd9d995 to
eab2c1b
Compare
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
033e271 to
12acd31
Compare
18 tasks
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 (3)
examples/configs/withprompushgateway/config.json (1)
33-39:⚠️ Potential issue | 🟠 MajorGemini key entry is missing required
name.Line 35-39 defines a provider key without
name, but key objects require it in the config schema. This example can fail validation.🛠️ Proposed fix
"gemini": { "keys": [ { + "name": "Gemini API Key", "value": "env.GEMINI_API_KEY", "weight": 1, "use_for_batch_api": true, "models": ["*"] } ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/configs/withprompushgateway/config.json` around lines 33 - 39, The provider key object inside the "keys" array is missing the required name property; update the key entry (the object with "value": "env.GEMINI_API_KEY", "weight": 1, "use_for_batch_api": true, "models": ["*"]) to include a "name" field (e.g., "name": "GEMINI_API_KEY") so the config conforms to the schema and the env reference matches the key name.helm-charts/bifrost/values.schema.json (1)
2736-2754:⚠️ Potential issue | 🟠 MajorLine 2753 description contradicts the standardized deny-by-default convention established in this commit.
Line 2738 (
allowed_models) correctly documents deny-by-default:"empty array denies all (deny-by-default)", but line 2753 (keys) still states the old behavior:"empty means all keys allowed". Update the description to align with the new convention:"keys": { "type": "array", - "description": "Provider keys for this config (empty means all keys allowed for this provider)", + "description": "Provider keys for this config. Use [\"*\"] or list specific key names to allow; empty array denies all (deny-by-default).",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm-charts/bifrost/values.schema.json` around lines 2736 - 2754, The "keys" property description contradicts the deny-by-default convention; update the description for the "keys" array (the schema entry named "keys") to match "allowed_models" by stating that an empty array denies all (deny-by-default) rather than "empty means all keys allowed"; ensure the text clearly mirrors the phrasing used for "allowed_models" so the behavior is documented consistently across the schema.transports/bifrost-http/server/server.go (1)
534-545:⚠️ Potential issue | 🟠 MajorDon't drop the wildcard signal before seeding the filtered model catalog.
These loops skip
*but never remember that a key was unrestricted. After that,models: ["*"]becomes indistinguishable frommodels: [], and mixed wildcard+restricted keys collapse to the restricted subset only. That seeds the filtered catalog with the wrong visibility, so/api/modelsand the Virtual Key sheet can under/over-report accessible models. Please carry a separatehasWildcardModelssignal here (or reuse the reducer behindfilterModelsByKeys) and only treat an empty explicit-model set as deny-all when no wildcard key was present.Also applies to: 769-779, 1261-1271
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/bifrost-http/server/server.go` around lines 534 - 545, The loop building modelsInKeys currently skips wildcard entries ("*") but never records that a provider had an unrestricted key, causing UpsertModelDataForProvider to lose the "allow-all" signal; update the code that iterates providerKeys (and the similar loops around where provider models are seeded) to track a boolean hasWildcardModels when any key.Models contains "*" and pass that information into the logic that computes the final model set before calling s.Config.ModelCatalog.UpsertModelDataForProvider (or alter UpsertModelDataForProvider call-site to include the wildcard flag), ensuring that an explicit models: ["*"] is treated as allow-all (not as empty/deny-all) and that mixed wildcard+restricted keys preserve the wildcard semantics.
♻️ Duplicate comments (2)
core/providers/mistral/models.go (1)
20-21:⚠️ Potential issue | 🟠 MajorHonor deny-by-default and wildcard semantics in filtering/backfill.
At Line 20 and Line 35,
len(allowedModels) > 0still makes empty lists behave as allow-all, and["*"]can be treated like a literal model token. That conflicts with the PR contract ([]deny-all,["*"]allow-all).🔧 Suggested fix
includedModels := make(map[string]bool) + allowAll := slices.Contains(allowedModels, "*") + denyAll := len(allowedModels) == 0 for _, model := range response.Data { - if !unfiltered && len(allowedModels) > 0 && !slices.Contains(allowedModels, model.ID) { + if !unfiltered && (denyAll || (!allowAll && !slices.Contains(allowedModels, model.ID))) { continue } bifrostResponse.Data = append(bifrostResponse.Data, schemas.Model{ @@ // Backfill allowed models that were not in the response - if !unfiltered && len(allowedModels) > 0 { + if !unfiltered && !allowAll && !denyAll { for _, allowedModel := range allowedModels { if !includedModels[allowedModel] { bifrostResponse.Data = append(bifrostResponse.Data, schemas.Model{Also applies to: 35-36
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@core/providers/mistral/models.go` around lines 20 - 21, The current filter uses "len(allowedModels) > 0" which treats [] as allow-all and doesn't treat ["*"] as wildcard; update the conditional around allowedModels/unfiltered (the block using allowedModels, unfiltered, model.ID and slices.Contains) to explicitly implement deny-by-default and wildcard semantics: when not unfiltered, first if len(allowedModels) == 0 then skip (deny-all), if len(allowedModels) == 1 && allowedModels[0] == "*" then allow all (do not skip), otherwise use slices.Contains(allowedModels, model.ID) to decide skipping; apply the same change to both occurrences referenced in the diff.transports/config.schema.json (1)
1517-1522:⚠️ Potential issue | 🟡 MinorReject empty strings in model/key identifier arrays.
Line 1521, Line 1536, and Line 1849 currently allow
"", which is not a meaningful model or key identifier. AddminLength: 1to each item schema.🛠️ Proposed fix
"allowed_models": { "type": "array", "description": "Allowed models for this provider config. Use [\"*\"] to allow all models; empty array denies all (deny-by-default).", "items": { - "type": "string" + "type": "string", + "minLength": 1 } }, @@ "key_ids": { "type": "array", "description": "Keys allowed for this provider config. Use [\"*\"] to allow all keys; empty array denies all (deny-by-default). In config.json, values are key names. Via the API, values are key UUIDs.", "items": { - "type": "string" + "type": "string", + "minLength": 1 } } @@ "models": { "type": "array", "items": { - "type": "string" + "type": "string", + "minLength": 1 }, "description": "Models this key can access. Use [\"*\"] to allow all models; empty array denies all (deny-by-default)." },Also applies to: 1532-1537, 1846-1851
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@transports/config.schema.json` around lines 1517 - 1522, The item schema for string arrays that represent model or key identifiers (e.g., "allowed_models") currently permits empty strings; update each such array item schema to include "minLength": 1 so empty strings are rejected. Locate the JSON schema entries for "allowed_models" and the other identifier arrays flagged in the review (the item schemas around the commented ranges) and add "minLength": 1 to their "items" object for type "string" so each array element must be at least one character long. Ensure you apply the same change to the other two similar array item schemas mentioned in the comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@examples/configs/withprompushgateway/config.json`:
- Around line 33-39: The provider key object inside the "keys" array is missing
the required name property; update the key entry (the object with "value":
"env.GEMINI_API_KEY", "weight": 1, "use_for_batch_api": true, "models": ["*"])
to include a "name" field (e.g., "name": "GEMINI_API_KEY") so the config
conforms to the schema and the env reference matches the key name.
In `@helm-charts/bifrost/values.schema.json`:
- Around line 2736-2754: The "keys" property description contradicts the
deny-by-default convention; update the description for the "keys" array (the
schema entry named "keys") to match "allowed_models" by stating that an empty
array denies all (deny-by-default) rather than "empty means all keys allowed";
ensure the text clearly mirrors the phrasing used for "allowed_models" so the
behavior is documented consistently across the schema.
In `@transports/bifrost-http/server/server.go`:
- Around line 534-545: The loop building modelsInKeys currently skips wildcard
entries ("*") but never records that a provider had an unrestricted key, causing
UpsertModelDataForProvider to lose the "allow-all" signal; update the code that
iterates providerKeys (and the similar loops around where provider models are
seeded) to track a boolean hasWildcardModels when any key.Models contains "*"
and pass that information into the logic that computes the final model set
before calling s.Config.ModelCatalog.UpsertModelDataForProvider (or alter
UpsertModelDataForProvider call-site to include the wildcard flag), ensuring
that an explicit models: ["*"] is treated as allow-all (not as empty/deny-all)
and that mixed wildcard+restricted keys preserve the wildcard semantics.
---
Duplicate comments:
In `@core/providers/mistral/models.go`:
- Around line 20-21: The current filter uses "len(allowedModels) > 0" which
treats [] as allow-all and doesn't treat ["*"] as wildcard; update the
conditional around allowedModels/unfiltered (the block using allowedModels,
unfiltered, model.ID and slices.Contains) to explicitly implement
deny-by-default and wildcard semantics: when not unfiltered, first if
len(allowedModels) == 0 then skip (deny-all), if len(allowedModels) == 1 &&
allowedModels[0] == "*" then allow all (do not skip), otherwise use
slices.Contains(allowedModels, model.ID) to decide skipping; apply the same
change to both occurrences referenced in the diff.
In `@transports/config.schema.json`:
- Around line 1517-1522: The item schema for string arrays that represent model
or key identifiers (e.g., "allowed_models") currently permits empty strings;
update each such array item schema to include "minLength": 1 so empty strings
are rejected. Locate the JSON schema entries for "allowed_models" and the other
identifier arrays flagged in the review (the item schemas around the commented
ranges) and add "minLength": 1 to their "items" object for type "string" so each
array element must be at least one character long. Ensure you apply the same
change to the other two similar array item schemas mentioned in the comment.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ec968b72-3627-410f-9ed9-95fda116335a
📒 Files selected for processing (34)
core/bifrost.gocore/changelog.mdcore/providers/mistral/mistral.gocore/providers/mistral/models.gocore/schemas/transcriptions.godocs/features/governance/routing.mdxexamples/configs/partial/config.jsonexamples/configs/withconfigstore/config.jsonexamples/configs/withlogstore/config.jsonexamples/configs/withpostgresmcpclientsinconfig/config.jsonexamples/configs/withprompushgateway/config.jsonexamples/configs/withvirtualkeys/config.jsonframework/changelog.mdframework/configstore/migrations.goframework/configstore/tables/key.goframework/configstore/tables/virtualkey.goframework/modelcatalog/main.gohelm-charts/bifrost/values.schema.jsonplugins/governance/main.goplugins/governance/resolver.goplugins/governance/resolver_test.gotransports/bifrost-http/handlers/governance.gotransports/bifrost-http/handlers/providers.gotransports/bifrost-http/lib/config.gotransports/bifrost-http/server/server.gotransports/changelog.mdtransports/config.schema.jsonui/app/workspace/providers/fragments/apiKeysFormFragment.tsxui/app/workspace/providers/views/providerKeyForm.tsxui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsxui/app/workspace/virtual-keys/views/virtualKeySheet.tsxui/components/ui/asyncMultiselect.tsxui/components/ui/modelMultiselect.tsxui/lib/types/schemas.ts
💤 Files with no reviewable changes (1)
- transports/bifrost-http/lib/config.go
✅ Files skipped from review due to trivial changes (2)
- core/changelog.md
- transports/changelog.md
🚧 Files skipped from review as they are similar to previous changes (12)
- plugins/governance/main.go
- docs/features/governance/routing.mdx
- plugins/governance/resolver_test.go
- framework/changelog.md
- framework/configstore/tables/virtualkey.go
- framework/configstore/tables/key.go
- plugins/governance/resolver.go
- transports/bifrost-http/handlers/governance.go
- ui/components/ui/modelMultiselect.tsx
- ui/app/workspace/virtual-keys/views/virtualKeyDetailsSheet.tsx
- core/schemas/transcriptions.go
- core/providers/mistral/mistral.go
Pratham-Mishra04
force-pushed
the
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
branch
from
12acd31 to
04a4831
Compare
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
eab2c1b to
112aee8
Compare
Collaborator
Author
Merge activity
|
Pratham-Mishra04
changed the base branch from
03-10-feat_vk_provider_config_key_ids_now_supports_wildcard_to_allow_all_keys_empty_key_ids_denies_all
to
graphite-base/2113
Pratham-Mishra04
force-pushed
the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
from
112aee8 to
1b7f3a0
Compare
Pratham-Mishra04
deleted the
03-16-feat_standardize_empty_array_conventions_in_bifrost
branch
akshaydeo
added a commit
that referenced
this pull request
Apr 18, 2026
* refactor: standardize empty array conventions for VK Provider & MCP Configs, and makes Provider Config weight optional for routing (#1932)
## Summary
Changes Virtual Key provider and MCP configurations from "allow-all by default" to "deny-by-default" security model. Virtual Keys now require explicit provider and MCP client configurations to allow access, improving security posture.
## Changes
- **Provider Configs**: Empty `provider_configs` now blocks all providers instead of allowing all
- **MCP Configs**: Empty `mcp_configs` now blocks all MCP tools instead of allowing all
- **Weight Field**: Changed provider `weight` from required `float64` to optional `*float64` - null weight excludes provider from weighted routing
- **Migration**: Added automatic backfill migration to preserve existing Virtual Key behavior by adding all available providers/MCP clients to VKs with empty configs
- **Documentation**: Updated all references to reflect new deny-by-default behavior
- **UI Updates**: Modified Virtual Key creation/editing interface to reflect new behavior and weight handling
## Type of change
- [x] Feature
- [x] Refactor
- [x] Documentation
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
Test Virtual Key creation and provider/MCP access:
```sh
# Core/Transports
go version
go test ./...
# Test Virtual Key with no provider configs blocks requests
curl -X POST http://localhost:8080/v1/chat/completions \
-H "Authorization: Bearer sk-bf-empty-vk" \
-H "Content-Type: application/json" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Should return error about no providers configured
# Test Virtual Key with provider configs allows requests
curl -X POST http://localhost:8080/v1/chat/completions \
-H "Authorization: Bearer sk-bf-configured-vk" \
-H "Content-Type: application/json" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Should work normally
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
## Breaking changes
- [x] Yes
**Impact**: Existing Virtual Keys with empty `provider_configs` or `mcp_configs` would be blocked after this change.
**Migration**: Automatic migration `migrationBackfillEmptyVirtualKeyConfigs` runs on startup to backfill existing Virtual Keys with all available providers/MCP clients, preserving current behavior. New Virtual Keys created after this change will use deny-by-default.
## Security considerations
This change significantly improves security posture by requiring explicit configuration of allowed providers and MCP tools for Virtual Keys. The automatic migration ensures no disruption to existing deployments while new Virtual Keys benefit from the more secure default behavior.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: add MCP auto tool injection toggle (#1933)
## Summary
Adds a new configuration option `DisableAutoToolInject` to the MCP (Model Context Protocol) system that allows disabling automatic tool injection into requests. When enabled, MCP tools are only included when explicitly requested via context headers or filters, providing more granular control over tool availability.
## Changes
- Added `DisableAutoToolInject` field to `MCPToolManagerConfig` schema with runtime update support
- Implemented atomic boolean storage in `ToolsManager` to safely handle concurrent access
- Added logic in `ParseAndAddToolsToRequest` to respect the disable flag and only inject tools when explicit context filters are present
- Extended configuration management with database migration, UI controls, and API endpoints
- Added hot-reload capability through `UpdateMCPDisableAutoToolInject` methods across the stack
- Updated UI with a toggle switch and clear documentation about the feature's behavior
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Validate the new MCP auto tool injection toggle:
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Test the feature:
1. Configure MCP clients and tools
2. Enable "Disable Auto Tool Injection" in the MCP configuration UI
3. Make requests without explicit tool headers - tools should not be injected
4. Make requests with `x-bf-mcp-include-tools` header - tools should be injected
5. Verify hot-reload works by toggling the setting without server restart
## Screenshots/Recordings
UI changes include a new toggle switch in the MCP configuration view with descriptive text explaining when tools are injected based on explicit headers.
## Breaking changes
- [ ] Yes
- [x] No
This is a backward-compatible addition with a default value of `false` (auto injection enabled).
## Related issues
This addresses the need for more granular control over MCP tool injection behavior in request processing.
## Security considerations
The feature provides better control over tool exposure by allowing administrators to require explicit opt-in for tool injection, potentially reducing unintended tool access.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: VK MCP config now works as an AllowList (#1940)
## Summary
This PR implements MCP tool governance by enforcing virtual key MCP configurations as an execution-time allow-list. When virtual keys have empty MCPConfigs, all MCP tools are denied. When non-empty, each tool is validated against the configured allow-list at both inference time and MCP tool execution.
## Changes
- **Context parameter updates**: Changed MCP-related functions to use `*schemas.BifrostContext` instead of `context.Context` to enable tool tracking
- **Tool tracking**: Added `BifrostContextKeyMCPAddedTools` context key to track which MCP tools are added to requests
- **Governance enforcement**: Virtual key MCP configurations now act as execution-time allow-lists with validation in both `PreMCPHook` and `evaluateGovernanceRequest`
- **Auto-injection control**: Added `DisableAutoToolInject` configuration option that respects the toggle and skips auto-injection when headers are already set by callers
- **Decision type**: Added `DecisionMCPToolBlocked` for MCP tool governance violations
- **UI improvements**: Updated MCP view description and sidebar item naming for better clarity
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Test MCP tool governance with virtual keys:
```sh
# Core/Transports
go version
go test ./...
# Test with virtual key having empty MCPConfigs (should deny all MCP tools)
curl -X POST /v1/chat/completions \
-H "x-bf-virtual-key: test-vk-empty-mcp" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Test with virtual key having specific MCP tool allowlist
curl -X POST /v1/chat/completions \
-H "x-bf-virtual-key: test-vk-with-mcp" \
-d '{"model": "gpt-4", "messages": [{"role": "user", "content": "test"}]}'
# Test disable auto tool inject configuration
curl -X PUT /v1/config/mcp/disable-auto-tool-inject \
-d '{"disable": true}'
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
New configuration options:
- `disable_auto_tool_inject`: Boolean flag to disable automatic MCP tool injection
- Virtual key `MCPConfigs`: Array of MCP client configurations that act as allow-lists
## Screenshots/Recordings
UI changes include updated MCP configuration view with clearer descriptions for the disable auto tool injection toggle and improved sidebar navigation labels.
## Breaking changes
- [x] Yes
- [ ] No
**Impact**: MCP-related function signatures now require `*schemas.BifrostContext` instead of `context.Context`. Virtual keys with empty MCPConfigs will now deny all MCP tools by default.
**Migration**: Update any custom MCP integrations to use the new context parameter type. Configure MCPConfigs on virtual keys that need MCP tool access.
## Related issues
Implements MCP tool governance and execution-time validation for virtual key configurations.
## Security considerations
- **Access control**: Virtual key MCP configurations now enforce strict allow-lists for tool execution
- **Context isolation**: Tool tracking is isolated per request context to prevent cross-request leakage
- **Validation**: Both pre-execution and execution-time validation prevent unauthorized tool access
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* refactor: standardize empty array conventions for VK Provider Config Allowed Keys (#2006)
## Summary
Migrates VK provider config allowed keys from implicit allow-all semantics to explicit deny-by-default behavior. Adds `AllowAllKeys` boolean field to enable granular key access control while maintaining backward compatibility.
## Changes
- Added `AllowAllKeys` boolean field to `TableVirtualKeyProviderConfig` with database migration
- Backfilled existing configs with `allow_all_keys=true` to preserve current behavior
- Updated key resolution logic: empty keys now denies all access, `["*"]` wildcard allows all keys
- Modified governance resolver to set empty `includeOnlyKeys` slice when no keys are configured
- Enhanced HTTP handlers to recognize `["*"]` wildcard and set `AllowAllKeys` flag appropriately
- Updated UI to display "Allow All Keys" option and show deny-by-default messaging
- Added JSON unmarshaling support for `["*"]` wildcard in config files
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Validate the migration and new key access control behavior:
```sh
# Core/Transports
go version
go test ./...
# Test migration runs successfully
go run main.go migrate
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Test scenarios:
1. Create VK with empty `key_ids` - should deny all keys
2. Create VK with `key_ids: ["*"]` - should allow all keys
3. Create VK with specific key IDs - should allow only those keys
4. Verify existing VKs maintain their current behavior after migration
## Screenshots/Recordings
UI now shows:
- "Allow All Keys" option in key selection dropdown
- "No keys allowed" vs "All keys allowed" status indicators
- "No providers configured (deny-by-default)" messaging
## Breaking changes
- [ ] Yes
- [x] No
The migration preserves existing behavior by setting `allow_all_keys=true` for configs that previously had no keys specified.
## Related issues
Part of VK access control enhancement initiative.
## Security considerations
Improves security posture by implementing deny-by-default semantics for key access. Existing deployments maintain current access patterns through automatic backfill migration.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* refactor: standardize empty array conventions for allowed models (#2113)
## Summary
Standardizes empty array conventions across Bifrost to implement deny-by-default security semantics. Previously, empty arrays for `allowed_models` and `Models` fields meant "allow all", creating potential security gaps. Now `["*"]` explicitly means "allow all" while empty arrays mean "deny all".
## Changes
- **Core Logic**: Updated model filtering in `bifrost.go` and `selectKeyFromProviderForModel` to treat empty `Models` arrays as deny-all and `["*"]` as allow-all
- **Database Migration**: Added `migrationBackfillAllowedModelsWildcard` to convert existing empty arrays to `["*"]` preserving current behavior for existing records
- **Model Catalog**: Updated `IsModelAllowedForProvider` to use wildcard semantics with deny-by-default fallback
- **Schema Defaults**: Changed default `Models` value from `[]` to `["*"]` in table definitions and form schemas
- **UI Components**: Enhanced `ModelMultiselect` with `allowAllOption` prop and updated virtual key forms to handle wildcard selection
- **Documentation**: Updated JSON schemas, comments, and tooltips to reflect new conventions
- **Governance**: Updated provider config filtering logic to use new wildcard semantics
- **Server Bootstrap**: Added wildcard filtering when loading models to prevent literal "*" from appearing as a model name
## Type of change
- [x] Refactor
- [ ] Bug fix
- [ ] Feature
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
Validate the migration and new semantics:
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Test scenarios:
1. Create new virtual keys - should default to `["*"]` for allowed models
2. Create new provider keys - should default to `["*"]` for models
3. Verify existing keys with empty arrays are migrated to `["*"]`
4. Test that empty arrays now deny all models/keys as expected
5. Verify UI shows "All models allowed" for wildcard and "No models (deny all)" for empty arrays
## Screenshots/Recordings
UI changes include:
- Model multiselect now shows "Allow All Models" option
- Virtual key details display "All Models" badge for wildcard vs "No models (deny all)" for empty
- Form placeholders updated to reflect new semantics
## Breaking changes
- [x] Yes
- [ ] No
**Migration Impact**: The database migration automatically converts existing empty `allowed_models` and `models_json` arrays to `["*"]`, preserving current behavior. However, any new configurations with empty arrays will now deny access instead of allowing all. Applications relying on "empty = allow all" semantics must be updated to use `["*"]` explicitly.
## Related issues
Part of security hardening initiative to implement explicit allow-lists and deny-by-default semantics across Bifrost configuration.
## Security considerations
This change significantly improves security posture by:
- Eliminating ambiguous "empty means allow all" semantics
- Implementing explicit deny-by-default for new configurations
- Requiring intentional wildcard usage via `["*"]` for broad access
- Maintaining backward compatibility through automatic migration
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* refactor: replace string slices with WhiteList for allowlist fields (#2125)
## Summary
Introduces a new `WhiteList` type to standardize whitelist behavior across the codebase, replacing manual slice operations and string comparisons with semantic methods for handling allow/deny lists.
## Changes
- Added `WhiteList` type with methods `IsAllowed()`, `IsUnrestricted()`, `IsEmpty()`, `Contains()`, and `Validate()`
- Replaced `[]string` fields with `WhiteList` for model restrictions, tool filtering, and key access controls
- Updated all whitelist logic to use semantic methods instead of manual `slices.Contains()` checks
- Added validation to ensure wildcards ("*") aren't mixed with specific values and prevent duplicates
- Improved case-insensitive matching for whitelist comparisons
## Type of change
- [x] Refactor
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Plugins
## How to test
Verify that whitelist behavior remains consistent across all affected components:
```sh
# Core/Transports
go version
go test ./...
# Test specific whitelist scenarios:
# - Empty lists deny all access
# - ["*"] allows all access
# - Specific lists only allow listed items
# - Mixed wildcards and specific items are rejected
# - Duplicate entries are rejected
```
Test key model filtering, MCP tool execution, and virtual key configurations to ensure whitelist logic works correctly.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
The `WhiteList` type maintains the same JSON serialization format as `[]string`, so existing configurations remain compatible.
## Related issues
N/A
## Security considerations
Improves security by standardizing deny-by-default behavior and adding validation to prevent misconfigured whitelists that could inadvertently grant excessive permissions.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: add request-level extra headers support for MCP tool execution (#2126)
## Summary
This PR adds support for request-level extra headers in MCP tool execution, allowing callers to forward specific headers to MCP servers at runtime based on a per-client allowlist configuration.
## Changes
- Added `AllowedExtraHeaders` field to MCP client configuration with allowlist semantics (empty array = deny all, `["*"]` = allow all)
- Introduced `BifrostContextKeyMCPExtraHeaders` context key to track headers forwarded to MCP tools
- Created `core/mcp/utils` package with `GetHeadersForToolExecution` function to merge static and dynamic headers
- Updated MCP tool execution in both regular tool manager and Starlark code mode to use the new header forwarding system
- Added database migration for `allowed_extra_headers_json` column in MCP client table
- Updated UI to include allowed extra headers configuration in MCP client management
- Enhanced auth demo server example to demonstrate tool-execution level authentication patterns
## Type of change
- [x] Feature
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] UI (Next.js)
## How to test
1. Configure an MCP client with allowed extra headers:
```json
{
"name": "test-client",
"connection_string": "http://localhost:3002/",
"auth_type": "headers",
"headers": {
"X-API-Key": "connection-secret"
},
"allowed_extra_headers": ["X-Tool-Token"],
"tools_to_execute": ["*"]
}
```
2. Make requests with extra headers that should be forwarded:
```bash
curl -X POST http://localhost:8080/v1/chat/completions \
-H "Authorization: Bearer your-key" \
-H "X-Tool-Token: tool-execution-secret" \
-d '{
"model": "gpt-4",
"messages": [{"role": "user", "content": "Use the secret_data tool"}],
"tools": [{"type": "function", "function": {"name": "secret_data"}}]
}'
```
3. Test the auth demo server:
```bash
cd examples/mcps/auth-demo-server
go run main.go
# Server demonstrates two-tier auth: connection-level (X-API-Key) and tool-level (X-Tool-Token)
```
4. Run tests:
```sh
go test ./core/mcp/...
go test ./transports/bifrost-http/...
cd ui
pnpm test
pnpm build
```
## Breaking changes
- [ ] Yes
- [x] No
This is a backward-compatible addition. Existing MCP clients will have empty `allowed_extra_headers` (deny all extra headers) which maintains current behavior.
## Security considerations
- Extra headers are filtered through a strict allowlist per MCP client
- Security denylist prevents auth header overrides via extra headers
- Two-tier authentication pattern demonstrated: connection-level + tool-execution level
- Headers are only forwarded to MCP servers that explicitly allow them
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* fix: apply MCP tool filtering headers to tools/list response when using bifrost as MCP gateway (#2127)
## Summary
Adds support for `x-bf-mcp-include-clients` and `x-bf-mcp-include-tools` request headers to filter MCP tools/list response when using Bifrost as an MCP gateway. This ensures that tool filtering is respected at the MCP protocol level, not just during inference.
## Changes
- Implemented dynamic tool filtering in MCP server handlers that respects per-request include headers
- Added `makeIncludeClientsFilter()` function that filters tools based on request context values
- Registered the tool filter on both global and virtual key MCP servers during initialization
- Updated documentation to clarify that `mcp-include-tools` requires `clientName-toolName` format
- Enhanced examples in documentation to show proper tool naming format
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [x] Docs
## How to test
Test MCP gateway functionality with tool filtering:
```sh
# Test tools/list filtering with include-tools header
curl --location 'http://localhost:8080/mcp/tools/list' \
--header 'x-bf-mcp-include-tools: gmail-send_email,filesystem-read_file' \
--header 'Authorization: Bearer your-vk-here'
# Test tools/list filtering with include-clients header
curl --location 'http://localhost:8080/mcp/tools/list' \
--header 'x-bf-mcp-include-clients: gmail,filesystem' \
--header 'Authorization: Bearer your-vk-here'
# Verify chat completions still respect the same headers
curl --location 'http://localhost:8080/v1/chat/completions' \
--header 'x-bf-mcp-include-tools: gmail-send_email' \
--header 'Content-Type: application/json' \
--data '{
"model": "openai/gpt-4o-mini",
"messages": [{"role": "user", "content": "What tools are available?"}]
}'
```
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
The tool filtering mechanism ensures that virtual key restrictions are properly enforced at the MCP protocol level, preventing unauthorized access to tools that should be filtered out based on request headers.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* refactor: parallelize model listing for providers to speed up startup time (#2151)
## Summary
Parallelizes model listing operations for providers during server startup and provider reloading to significantly reduce initialization time. Previously, model listing was performed sequentially for each provider, causing slower startup times especially when multiple providers were configured.
## Changes
- Added concurrent execution using goroutines and sync.WaitGroup for model listing operations in three key functions: `ReloadProvider`, `ForceReloadPricing`, and `Bootstrap`
- In `ReloadProvider`, both filtered and unfiltered model listing requests now run concurrently for the same provider
- In `ForceReloadPricing` and `Bootstrap`, model listing for different providers now runs in parallel instead of sequentially
- Moved provider key retrieval earlier in `ReloadProvider` to ensure it happens before concurrent model listing
- Added proper context cancellation with defer statements for bifrost contexts
## Type of change
- [x] Refactor
## Affected areas
- [x] Transports (HTTP)
## How to test
Test server startup time with multiple providers configured to verify the performance improvement:
```sh
# Core/Transports
go version
go test ./...
# Test with multiple providers configured
# Measure startup time before and after the change
time go run main.go
```
Configure multiple providers in your bifrost configuration and observe faster startup times, especially noticeable when providers have high latency or many models.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications. The change maintains the same authentication and authorization patterns while improving performance through parallelization.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* fix: reorder migrations and set AllowAllKeys to true for virtual key provider configs (#2158)
## Summary
Fixes database migration ordering issue and ensures virtual key configurations are properly initialized with the AllowAllKeys field set to true.
## Changes
- Reordered database migrations to execute `migrationAddAllowAllKeysToProviderConfig` before `migrationBackfillEmptyVirtualKeyConfigs` to ensure the AllowAllKeys column exists before backfilling
- Added `AllowAllKeys: true` to provider configurations created during virtual key backfill migration to enable unrestricted key access by default
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that database migrations run successfully and virtual key configurations are created with proper defaults:
```sh
# Core/Transports
go version
go test ./...
```
Test migration ordering by running against a fresh database to ensure no column reference errors occur.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
This change enables unrestricted key access by default for virtual key configurations, which may have security implications depending on the intended access control model.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: implement scoped pricing override
* refactor: custom pricing refactor
* fix: resolve merge conflicts in config loading and governance functions (#2230)
## Summary
Resolves Git merge conflicts in the bifrost-http configuration loading code by cleaning up duplicate function definitions and consolidating the configuration initialization flow.
## Changes
- Removed Git merge conflict markers and duplicate code blocks from `LoadConfig` function
- Consolidated governance configuration loading by keeping both `loadGovernanceConfigFromFile` and `loadGovernanceConfig` functions with distinct purposes
- Removed duplicate `convertSchemasMCPClientConfigToTable` function definition
- Moved pricing overrides initialization logic to `initFrameworkConfig` function for better organization
- Cleaned up extensive duplicate default configuration loading code that was causing merge conflicts
- Changed error handling for pricing overrides from returning error to logging warning
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that configuration loading works correctly without merge conflicts:
```sh
# Core/Transports
go version
go test ./...
go build ./transports/bifrost-http/...
```
Test configuration loading with various scenarios:
- Config file present
- Config file absent (default loading)
- Store-based configuration
- Governance and MCP configuration loading
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications - this is a merge conflict resolution that maintains existing functionality.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: add Stability AI model support for Bedrock image generation (#2180)
## Summary
Adds support for Stability AI image generation models (stability.stable-image-*) to the Bedrock provider, enabling text-to-image generation with models like stability.stable-image-core-v1:1 and stability.stable-image-ultra-v1:1.
## Changes
- Added `isStabilityAIModel()` function to detect Stability AI models by "stability." prefix
- Created `ToStabilityAIImageGenerationRequest()` to convert Bifrost requests to Stability AI's flat request format
- Implemented `StabilityAIImageGenerationRequest` type with support for prompt, mode, aspect_ratio, output_format, seed, and negative_prompt parameters
- Added conditional routing in `ImageGeneration()` to use Stability AI request format when appropriate
- Extended known fields for image generation parameters to include "aspect_ratio" and "input_images"
- Updated documentation comment to reflect Stability AI model support
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test Stability AI image generation through the Bedrock provider:
```sh
# Core/Transports
go version
go test ./...
# Test with a Stability AI model
curl -X POST http://localhost:8080/v1/images/generations \
-H "Content-Type: application/json" \
-H "Authorization: Bearer your-key" \
-d '{
"model": "stability.stable-image-core-v1:1",
"prompt": "A beautiful sunset over mountains",
"aspect_ratio": "16:9",
"output_format": "PNG"
}'
```
Ensure AWS credentials are configured for Bedrock access and the Stability AI models are available in your region.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No additional security implications beyond existing Bedrock provider authentication and AWS credential handling.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: add Stability AI image edit models support to Bedrock provider (#2225)
## Summary
Adds support for Stability AI image editing models in the Bedrock provider, expanding image editing capabilities beyond the existing Titan and Nova Canvas models.
## Changes
- Added `getStabilityAIEditTaskType()` function to infer edit task types from Stability AI model names (inpaint, outpaint, recolor, search-replace, erase-object, remove-bg, control-sketch, control-structure, style-guide, style-transfer, upscale-creative, upscale-conservative, upscale-fast)
- Created `ToStabilityAIImageEditRequest()` function to convert Bifrost requests to Stability AI's flat JSON format, with task-specific field validation
- Added `StabilityAIImageEditRequest` struct with comprehensive field support for all Stability AI edit operations
- Enhanced `BedrockImageGenerationResponse` with Seeds and FinishReasons fields for Stability AI compatibility
- Modified `ImageEdit()` method to route requests to appropriate conversion function based on model type
- Updated documentation to reflect expanded model support
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test with various Stability AI edit models through the Bedrock provider:
```sh
# Core/Transports
go version
go test ./...
# Test image editing with Stability AI models
# Example: stable-image-inpaint, stable-outpaint, stable-creative-upscale, etc.
```
Verify that task-specific parameters are correctly mapped and invalid fields are filtered out based on the detected task type.
## Screenshots/Recordings
N/A - Backend functionality only
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
Image data is handled as base64-encoded strings. Mask and image parameters are properly validated before processing.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* fix: send back accumulated usage in MCP agent mode (#2246)
## Summary
This PR fixes token usage tracking in MCP agent mode by accumulating usage across all LLM calls in the agent loop and returning the total usage in the final response.
## Changes
- Added usage accumulation logic in the MCP agent execution loop to track token consumption across multiple LLM calls
- Implemented `mergeUsage` function to combine token counts and costs from multiple `BifrostLLMUsage` values, handling all detail sub-fields including prompt tokens, completion tokens, and cost breakdowns
- Extended agent API adapters with `extractUsage` and `applyUsage` methods to handle usage extraction and application for both Chat API and Responses API
- Applied accumulated usage to the final response before returning it to the client
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test MCP agent mode with multiple tool calls to verify usage accumulation:
```sh
# Core/Transports
go version
go test ./...
# Test MCP agent mode with multiple LLM calls
# Verify that the returned usage reflects the sum of all calls in the agent loop
# Check that both token counts and cost details are properly accumulated
```
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications - this change only affects usage tracking and reporting.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* [codemode]: fixing string escape corruption, enable top-level control flow in starlark, refining the prompt of executecode tool (#2206)
## Changes
- **Enhanced Starlark dialect configuration**: Enabled top-level control flow statements (if/for/while), while loops, set() builtin, global variable reassignment, and recursive functions for a more Python-like experience
- **Improved string escape handling**: Removed automatic `\n` to newline conversion, allowing Starlark's native string escape processing to handle `\n`, `\t`, and other escape sequences correctly
- **Updated tool description**: Streamlined the executeToolCode tool description with clearer syntax notes, explicit documentation of Starlark differences from Python (no try/except, no classes, no imports, no f-strings), and emphasis on fresh isolated scope per execution
- **Enhanced error hints**: Added specific error messages for unsupported Python features like try/except/finally/raise, with guidance on alternative approaches and scope persistence warnings
- **Comprehensive test coverage**: Added tests for dialect options, string escape preservation, unsupported feature detection, and end-to-end JSON deserialization scenarios
## Type of change
- [ ] Feature
- [ ] Bug fix
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go) - Starlark CodeMode improvements
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Test the enhanced Starlark features with MCP CodeMode:
```sh
# Test dialect options (top-level control flow, while loops, etc.)
make test-mcp TESTCASE=TestStarlarkDialectOptions
# Test string escape handling
make test-mcp PATTERN=TestStarlarkStringEscape
# Test unsupported feature detection
make test-mcp PATTERN=TestStarlarkUnsupportedFeatures
```
## Breaking changes
- [ ] Yes
- [x] No
The Starlark changes are additive and maintain backward compatibility while enabling more Python-like syntax.
## Security considerations
Starlark CodeMode maintains its existing sandboxing with no additional network or filesystem access. The dialect enhancements only affect language features within the existing security boundary.
* logging in plugins (#2215)
## Summary
Reorders middleware initialization in the Bifrost HTTP server to ensure tracing middleware is added before transport interceptor middleware in the inference pipeline.
## Changes
- Moved tracing middleware initialization and setup earlier in the bootstrap process
- Reordered middleware registration so tracing middleware is prepended before transport interceptor middleware
- Updated comments to clarify the middleware ordering logic and rationale
The change ensures that tracing context and trace IDs are properly established before other middleware components process requests.
## Type of change
- [x] Bug fix
- [ ] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that tracing middleware executes before transport interceptor middleware by checking trace logs and middleware execution order.
```sh
# Core/Transports
go version
go test ./...
```
Test with tracing enabled to ensure trace IDs are properly set in context before subsequent middleware processing.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
No security implications - this is a middleware ordering change that affects observability components.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* fix: handling text, vtt, srt response format for transcriptions (#2102)
* feat: add virtual key access management for MCP clients (#2255)
## Summary
Adds virtual key access management to MCP client configuration, allowing administrators to control which virtual keys can access specific MCP servers and which tools they can execute on a per-VK basis.
## Changes
- Added `vk_configs` field to MCP client update API that accepts an array of virtual key configurations
- Each VK config specifies a virtual key ID and the tools it's allowed to execute on that MCP server
- When `vk_configs` is provided, it atomically replaces all existing VK assignments for the MCP client
- Added database method `GetVirtualKeyMCPConfigsByMCPClientID` to retrieve VK configs by MCP client
- Updated OpenAPI documentation to describe the new VK configuration functionality
- Enhanced UI with virtual key access management section in the MCP client sheet
- Added Go SDK context keys for MCP tool filtering: `MCPContextKeyIncludeClients`, `MCPContextKeyIncludeTools`, and `BifrostContextKeyMCPExtraHeaders`
- Updated context keys documentation with comprehensive MCP configuration examples
## Type of change
- [x] Feature
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] UI (Next.js)
- [x] Docs
## How to test
1. Create an MCP client with tools available
2. Create virtual keys in the system
3. Update the MCP client with VK configurations:
```sh
curl -X PUT /api/mcp/client/{id} \
-H "Content-Type: application/json" \
-d '{
"name": "test-client",
"vk_configs": [
{
"virtual_key_id": "vk-123",
"tools_to_execute": ["*"]
},
{
"virtual_key_id": "vk-456",
"tools_to_execute": ["read_file", "write_file"]
}
]
}'
```
4. Verify VK assignments are created/updated in the database
5. Test the UI by opening an MCP client sheet and managing virtual key access
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
## Screenshots/Recordings
The UI now includes a "Virtual Key Access" section in the MCP client configuration sheet where administrators can:
- Add virtual keys to grant access to the MCP server
- Configure which specific tools each virtual key can execute
- Remove virtual key access entirely
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
This enables fine-grained access control for MCP servers at the virtual key level, complementing the existing governance and budgeting features.
## Security considerations
- VK access controls are enforced through the governance plugin during MCP tool execution
- The atomic replacement of VK assignments prevents partial updates that could leave the system in an inconsistent state
- Tool-level restrictions allow principle of least privilege by limiting which MCP tools each virtual key can access
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: adds option to allow MCP clients to run on all virtual keys (#2258)
## Summary
Adds a new `AllowOnAllVirtualKeys` configuration option for MCP clients that enables them to be accessible to all virtual keys without requiring explicit per-key assignment. When enabled, all tools from the MCP client are available to every virtual key.
## Changes
- Added `AllowOnAllVirtualKeys` boolean field to `MCPClientConfig` schema and database table
- Updated MCP client manager to handle the new field during client updates
- Modified governance plugin to check for clients with `AllowOnAllVirtualKeys` enabled and automatically include their tools for all virtual keys
- Added database migration to add the new column to `TableMCPClient`
- Updated UI to include a toggle for the new setting with tooltip explanation
- Added OpenAPI documentation for the new field
- Updated configuration store methods to persist and retrieve the new field
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
1. Create or update an MCP client with `allow_on_all_virtual_keys: true`
2. Verify that the client's tools are available to all virtual keys without explicit assignment
3. Test that the governance plugin correctly allows tools from such clients
4. Verify the UI toggle works correctly in the MCP client edit sheet
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
The new configuration field `allow_on_all_virtual_keys` defaults to `false` to maintain backward compatibility.
## Screenshots/Recordings
If UI changes, add before/after screenshots or short clips.
## Breaking changes
- [ ] Yes
- [x] No
This is a backward-compatible addition with the new field defaulting to `false`.
## Related issues
Link related issues and discussions. Example: Closes #123
## Security considerations
This feature reduces access control granularity by allowing MCP clients to bypass virtual key restrictions when enabled. Administrators should carefully consider which MCP clients should have this permission as it grants broad access across all virtual keys.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* feat: add provider keys CRUD to configstore and in-memory store (#2159)
## Summary
Adds dedicated CRUD operations for individual provider keys at the data layer
(configstore interface + RDB implementation) and in-memory store. This enables
key-level operations without replacing the entire provider key set, which is
required for the new `/api/providers/{provider}/keys/*` endpoints.
## Changes
- Added `GetProviderKeys`, `GetProviderKey`, `CreateProviderKey`,
`UpdateProviderKey`, `DeleteProviderKey` to `ConfigStore` interface
- Implemented all five methods in `RDBConfigStore` with proper GORM queries,
error handling, and `ErrNotFound` propagation
- Extracted `schemaKeyFromTableKey` and `tableKeyFromSchemaKey` helpers to
deduplicate key conversion logic (previously inlined in `GetProvidersConfig`
and `GetProviderConfig`)
- Added `AddProviderKey`, `UpdateProviderKey`, `RemoveProviderKey` to in-memory
`Config` with mutex locking, DB persistence, and rollback on client update
failure
- Added `GetProviderKeysRaw`, `GetProviderKeysRedacted`, `GetProviderKeyRaw`,
`GetProviderKeyRedacted` read methods
- Added `TestProviderKeyCRUD` and `TestProviderKeyCRUD_ProviderMustExist`
integration tests
- Updated `MockConfigStore` with all five new interface methods
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
```sh
# Run configstore tests
go test ./framework/configstore/... -v -run TestProviderKeyCRUD
# Run config tests (mock store)
go test ./transports/bifrost-http/lib/... -v
```
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
Key values are handled through existing redaction infrastructure. No new secret
exposure paths introduced.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: add provider keys HTTP handlers and refactor optional keys (#2160)
## Summary
Adds HTTP handlers for the dedicated provider keys CRUD endpoints and removes
`keys` from provider API responses and payloads. Keys are now exclusively
managed via `/api/providers/{provider}/keys/*`. Also fixes a context timeout bug
in `ReloadProvider` where model discovery could exhaust the shared context
budget, causing subsequent DB calls to fail.
## Changes
### Provider keys handlers (`provider_keys.go`)
- New file with five handlers: `listProviderKeys`, `getProviderKey`,
`createProviderKey`, `updateProviderKey`, `deleteProviderKey`
- Includes `mergeUpdatedKey` (redacted value preservation logic used by
`updateProviderKey`)
- Key handlers enforce keyless provider validation and trigger model discovery
after mutations
### Provider handlers cleanup (`providers.go`)
- Registered new key routes: `GET/POST /api/providers/{provider}/keys`,
`GET/PUT/DELETE /api/providers/{provider}/keys/{key_id}`
- Extracted inline anonymous structs into named `providerCreatePayload` and
`providerUpdatePayload` types (without `Keys` field)
- Removed `Keys` field from `ProviderResponse`
- Switched `addProvider` from `json.Unmarshal` to `sonic.Unmarshal`
- Removed `oldConfigRedacted` fetch and the entire key merge block
(`mergeKeys`, `hasKeys`, `slices` usage) from `updateProvider`
- Removed `Keys` from `getProviderResponseFromConfig` response builder
- Removed unused `encoding/json` import
### Context timeout fix (`server.go`)
- Split shared `bfCtx` in `ReloadProvider` into separate contexts:
`filteredBfCtx` (15s) for filtered `ListModelsRequest` and `unfilteredBfCtx`
(fresh 15s) for unfiltered `ListModelsRequest`, each cancelled after use
- Changed `GetKeysByProvider` to use `context.Background()` since it's a local
DB call that shouldn't be gated by model discovery timeouts
- Added `hasNoKeys` check to emit warn-level logs instead of errors when model
discovery fails because no keys are configured
- Read in-memory key count via `GetProviderKeysRaw` for the `hasNoKeys` check
### Tests (`providers_test.go`)
- Cleared file (contained only tests for removed inline struct decoding)
## Type of change
- [x] Feature
- [x] Bug fix
- [x] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
```sh
# Build
go build ./transports/bifrost-http/...
# Manual: start Bifrost, then test key CRUD
curl -X POST localhost:8080/api/providers/openai/keys -d '{"name":"test-key","value":"sk-test"}'
curl localhost:8080/api/providers/openai/keys
curl -X PUT localhost:8080/api/providers/openai/keys/{key_id} -d '{"name":"updated","value":"sk-new"}'
curl -X DELETE localhost:8080/api/providers/openai/keys/{key_id}
# Verify provider endpoints no longer return keys
curl localhost:8080/api/providers/openai | jq 'has("keys")' # should be false
```
## Screenshots/Recordings
N/A
## Breaking changes
- [x] Yes
- [ ] No
Provider API responses no longer include `keys` field. Provider create/update
payloads no longer accept `keys`. Clients must use the dedicated
`/api/providers/{provider}/keys/*` endpoints for key management.
## Related issues
N/A
## Security considerations
- Key handlers use existing redaction infrastructure (`GetProviderKeyRedacted`)
before returning responses
- Keyless provider validation prevents key creation on providers that don't
support keys
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: migrate frontend to dedicated provider keys API (#2161)
## Summary
Migrates the frontend from reading provider keys via `provider.keys` (removed
from provider API response in PR #2160) to the dedicated `getProviderKeys`
query and `/api/keys` endpoint. Removes `keys` from all provider TypeScript
types. Key mutations patch caches from authoritative server responses; provider
updates invalidate the `ProviderKeys` tag to refresh key statuses after model
discovery. Also adds a read-only routing rule info sheet.
## Changes
### Types (`config.ts`, `schemas.ts`)
- Removed `keys` field from `ModelProviderConfig`, `AddProviderRequest`, and
`UpdateProviderRequest`
- Added `CreateProviderKeyRequest`, `UpdateProviderKeyRequest`,
`ListProviderKeysResponse` types
### Store (`providersApi.ts`, `baseApi.ts`)
- Added `ProviderKeys` tag type to `baseApi`
- Changed `getProviderKeys`/`getProviderKey` from `Providers` tag to
`ProviderKeys` tag (avoids invalidating provider cache on key changes)
- Added `invalidatesTags: [ProviderKeys, DBKeys]` on `updateProvider` mutation
(refreshes key statuses after model discovery)
- Removed `getProvider`/`getProviders` cache patches from `createProviderKey`,
`updateProviderKey`, `deleteProviderKey` (providers no longer carry keys)
- Added duplicate-check guards on `createProviderKey` cache patches to prevent
ghost keys
- Each key mutation patches `getProviderKeys` and `getAllKeys` caches from
authoritative server response
### Components
- **`modelProviderKeysTableView.tsx`**: Already uses `useGetProviderKeysQuery`;
formatting/indentation fixes
- **`page.tsx`**: Removed `keys: []` from fallback provider object and
`createProvider` call; simplified `KeyDiscoveryFailedBadge` to only check
provider-level status (removed per-key status check since keys are no longer
on provider)
- **`routingRuleSheet.tsx`**: `TargetRow` now receives `allKeys` prop (from
`useGetAllKeysQuery`) instead of `providersData` with `.keys`; filters keys
by target provider
- **`routingRuleInfoSheet.tsx`**: New read-only sheet component that displays
routing rule details (conditions, targets with provider icons and weight bars,
fallback chain, scope, priority, timestamps)
- **`settingsPanel.tsx`**: Uses `useGetAllKeysQuery` to determine configured
providers (replaces `p.keys.length > 0` check) and derive
`providerKeyConfigs` per provider
### Other frontend changes (from prior commit, unchanged)
- Added `getProviderKeys`, `getProviderKey` RTK Query endpoints
- Added `createProviderKey`, `updateProviderKey`, `deleteProviderKey` mutations
- Added `buildProviderUpdatePayload` utility for key-free provider updates
- Migrated `providerKeyForm.tsx` to separate create/update mutations
- Updated `addNewKeySheet.tsx` props from `keyIndex` to `keyId`
- Updated all 6 provider form fragments to use `buildProviderUpdatePayload`
- Removed dead `selectedProvider.keys` sync matchers from `providerSlice.ts`
## Type of change
- [x] Feature
- [x] Refactor
- [ ] Bug fix
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
```sh
cd ui
npm run build
npm run lint
```
Manual testing:
1. Navigate to Providers page, select a provider with keys
2. Verify keys table loads correctly from dedicated API
3. Create a new key — verify it appears immediately (no ghost/duplicate)
4. Toggle enable/disable — verify switch updates immediately
5. Edit a key — verify form pre-populates, save works
6. Delete a key — verify it disappears immediately
7. Update provider settings — verify key statuses refresh after save
8. Check sidebar badge shows provider-level discovery failures
9. Open Playground settings — verify provider/key dropdowns work
10. Open Routing Rules — verify target key selector works
11. Click a routing rule row — verify info sheet opens with correct details
(conditions, targets, fallbacks, scope, priority)
## Screenshots/Recordings
N/A — no visual changes to existing features; routing rule info sheet is new.
## Breaking changes
- [ ] Yes
- [x] No
Frontend-only changes consuming the new API shape from PR #2160.
## Related issues
N/A
## Security considerations
No new security considerations. Key values continue to be handled through
existing redaction on the backend.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* refactor: replace string slice with WhiteList type for model restrictions (#2282)
## Summary
Refactored model access control logic by replacing string slice with a dedicated `WhiteList` type for the `Models` field in `TableKey`. This change introduces a more structured approach to handling wildcard permissions and improves code readability.
## Changes
- Changed `Models` field type from `[]string` to `schemas.WhiteList` in `TableKey` struct
- Replaced manual wildcard checking (`model == "*"`) with `IsUnrestricted()` method calls across multiple functions
- Added missing mock method `GetVirtualKeyMCPConfigsByMCPClientIDs` to test configuration store
- Applied the refactoring consistently in `ReloadProvider`, `ForceReloadPricing`, and `Bootstrap` methods
## Type of change
- [x] Refactor
- [ ] Bug fix
- [ ] Feature
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that model access control continues to work correctly with both wildcard and specific model permissions:
```sh
# Core/Transports
go version
go test ./...
# Test specific areas affected by the changes
go test ./framework/configstore/tables/...
go test ./transports/bifrost-http/...
```
Test scenarios should include:
- Keys with wildcard permissions (`["*"]`)
- Keys with specific model restrictions
- Keys with empty model lists (deny-by-default behavior)
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
This refactoring maintains the existing security model for API key permissions. The deny-by-default behavior and wildcard functionality remain unchanged, just implemented through a more structured type system.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: add Plus icon and responsive text to pricing override create button (#2285)
## Summary
Improves the visual design and mobile responsiveness of the pricing overrides section by adding a Plus icon to the create button and optimizing the button text for different screen sizes.
## Changes
- Added Plus icon import from lucide-react
- Enhanced the "Create Override" button with a Plus icon and responsive text that shows "New Override" on larger screens and hides text on mobile
- Adjusted container spacing by removing top margin and changing flex alignment from `items-start` to `items-center` for better visual balance
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [ ] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [ ] Docs
## How to test
Navigate to the custom pricing overrides page and verify:
1. The "New Override" button displays with a Plus icon
2. On mobile screens, only the Plus icon is visible
3. On larger screens (sm and above), both icon and "New Override" text are visible
4. The button functionality remains unchanged when clicked
```sh
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
## Screenshots/Recordings
Before/after screenshots showing the button design changes and responsive behavior would be helpful.
## Breaking changes
- [x] Yes
- [ ] No
## Related issues
## Security considerations
No security implications - this is a purely visual enhancement.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* refactor: blacklist models on new convention (#2305)
## Summary
Implements comprehensive blacklist support for model filtering across all providers. This adds the ability to explicitly deny access to specific models at the key level, with blacklist rules taking precedence over allowlist rules.
## Changes
- Added `BlackList` type with semantic validation (supports wildcard "*" for block-all)
- Updated key selection logic to check both allowlist and blacklist constraints
- Modified all provider model listing functions to filter out blacklisted models
- Enhanced UI to support blacklist configuration with improved UX for wildcard selection
- Added blacklist filtering to model catalog and provider handlers
- Updated test cases to verify blacklist functionality
Key design decisions:
- Blacklist always wins over allowlist when conflicts occur
- Wildcard "*" in blacklist blocks all models for that key
- Empty blacklist blocks nothing (permissive default)
- Consistent filtering logic across all providers (Anthropic, Azure, Bedrock, Cohere, etc.)
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [x] Transports (HTTP)
- [x] Providers/Integrations
- [ ] Plugins
- [x] UI (Next.js)
- [x] Docs
## How to test
Test blacklist functionality with provider keys:
```sh
# Core/Transports
go version
go test ./...
# UI
cd ui
pnpm i || npm i
pnpm test || npm test
pnpm build || npm run build
```
Example configuration to test:
```json
{
"keys": [{
"id": "test-key",
"models": ["*"],
"blacklisted_models": ["gpt-4", "claude-3"]
}]
}
```
Verify that blacklisted models are excluded from model listings and key selection.
## Screenshots/Recordings
UI now shows "Blocked Models" field with improved tooltips and wildcard handling for denying access to specific models.
## Breaking changes
- [ ] Yes
- [x] No
The `blacklisted_models` field was already present in the schema but not fully implemented. This change makes it functional without breaking existing configurations.
## Related issues
Enhances model access control capabilities for fine-grained permission management.
## Security considerations
Improves security by allowing explicit denial of access to sensitive or expensive models at the key level. Blacklist rules cannot be bypassed by allowlist configurations.
## Checklist
- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
* minor fix add blacklisted model field in tableKeyFromSchemaKey (#2324)
## Summary
This PR adds support for the `BlacklistedModels` field when converting schema keys to table keys in the configuration store's RDB implementation.
## Changes
- Added `BlacklistedModels: key.BlacklistedModels` field mapping in the `tableKeyFromSchemaKey` function
- Ensures that blacklisted model information is properly preserved when converting between schema and table representations
## Type of change
- [ ] Bug fix
- [x] Feature
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [ ] Plugins
- [ ] UI (Next.js)
- [ ] Docs
## How to test
Verify that configuration keys with blacklisted models are properly stored and retrieved from the RDB configstore.
```sh
# Core/Transports
go version
go test ./...
```
Test creating configuration entries with `BlacklistedModels` specified and ensure they persist correctly through the RDB layer.
## Screenshots/Recordings
N/A
## Breaking changes
- [ ] Yes
- [x] No
## Related issues
N/A
## Security considerations
None - this change only adds field mapping for existing blacklisted models functionality.
## Checklist
- [ ] I read `docs/contributing/README.md` and followed the guidelines
- [ ] I added/updated tests where appropriate
- [ ] I updated documentation where needed
- [ ] I verified builds succeed (Go and UI)
- [ ] I verified the CI pipeline passes locally if applicable
* feat: add image edit input view on logs (#2321)
## Summary
Adds support for logging image edit and image variation requests by introducing new database columns and UI components to track and display these image manipulation operations alongside existing image generation functionality.
## Changes
- Added `image_edit_input` and `image_variation_input` columns to the logs table with corresponding database migrations
- Extended the Log struct with new fields for storing and parsing image edit/variation input data
- Updated logging plugin to capture image edit and variation request data with large payload threshold handling
- Enhanced UI to display input images and prompts for image edit operations and input images for variation operations
- Added image MIME type detection for proper display of base64-encoded images in the UI
## Type of change
- [x] Feature
- [ ] Bug fix
- [ ] Refactor
- [ ] Documentation
- [ ] Chore/CI
## Affected areas
- [x] Core (Go)
- [ ] Transports (HTTP)
- [ ] Providers/Integrations
- [x] Plugins
- [x] UI (Next.js)
- [ ] Do…
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
2 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
Standardizes empty array conventions across Bifrost to implement deny-by-default security semantics. Previously, empty arrays for
allowed_modelsandModelsfields meant "allow all", creating potential security gaps. Now["*"]explicitly means "allow all" while empty arrays mean "deny all".Changes
bifrost.goandselectKeyFromProviderForModelto treat emptyModelsarrays as deny-all and["*"]as allow-allmigrationBackfillAllowedModelsWildcardto convert existing empty arrays to["*"]preserving current behavior for existing recordsIsModelAllowedForProviderto use wildcard semantics with deny-by-default fallbackModelsvalue from[]to["*"]in table definitions and form schemasModelMultiselectwithallowAllOptionprop and updated virtual key forms to handle wildcard selectionType of change
Affected areas
How to test
Validate the migration and new semantics:
Test scenarios:
["*"]for allowed models["*"]for models["*"]Screenshots/Recordings
UI changes include:
Breaking changes
Migration Impact: The database migration automatically converts existing empty
allowed_modelsandmodels_jsonarrays to["*"], preserving current behavior. However, any new configurations with empty arrays will now deny access instead of allowing all. Applications relying on "empty = allow all" semantics must be updated to use["*"]explicitly.Related issues
Part of security hardening initiative to implement explicit allow-lists and deny-by-default semantics across Bifrost configuration.
Security considerations
This change significantly improves security posture by:
["*"]for broad accessChecklist
docs/contributing/README.mdand followed the guidelines