Skip to content

feat: add AllowAllKeys flag and fix virtual key config semantics #2008

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Pratham-Mishra04 wants to merge 1 commit into from
Closed

feat: add AllowAllKeys flag and fix virtual key config semantics #2008

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions framework/configstore/clientconfig.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ type VirtualKeyProviderConfigHashInput struct {
Provider string
Weight *float64
AllowedModels []string
AllowAllKeys bool // Distinguishes deny-all (false, no KeyIDs) from allow-all (true, no KeyIDs)
BudgetID *string
RateLimitID *string
KeyIDs []string // Only key IDs, not full key objects
Expand Down Expand Up @@ -670,6 +671,7 @@ func GenerateVirtualKeyHash(vk tables.TableVirtualKey) (string, error) {
Provider: pc.Provider,
Weight: pc.Weight,
AllowedModels: sortedAllowedModels,
AllowAllKeys: pc.AllowAllKeys,
BudgetID: pc.BudgetID,
RateLimitID: pc.RateLimitID,
KeyIDs: keyIDs,
Expand Down
12 changes: 6 additions & 6 deletions framework/configstore/tables/encryption_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import (
"gorm.io/driver/postgres"
"gorm.io/driver/sqlite"
"gorm.io/gorm"
"gorm.io/gorm/logger"
gormLogger "gorm.io/gorm/logger"
)

const testEncryptionKey = "test-encryption-key-for-testing-32bytes"
Expand All @@ -25,7 +25,7 @@ func init() {
func setupTestDB(t *testing.T) *gorm.DB {
t.Helper()
db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{
Logger: logger.Default.LogMode(logger.Silent),
Logger: gormLogger.Default.LogMode(gormLogger.Silent),
})
require.NoError(t, err)

Expand Down Expand Up @@ -1268,9 +1268,9 @@ func TestTableMCPClient_EncryptionDisabled_StoresPlaintext(t *testing.T) {
db := setupTestDB(t)

client := &TableMCPClient{
ClientID: "mcp-dis-1",
Name: "disabled-mcp",
ConnectionType: "sse",
ClientID: "mcp-dis-1",
Name: "disabled-mcp",
ConnectionType: "sse",
ConnectionString: schemas.NewEnvVar("https://mcp.example.com"),
Headers: map[string]schemas.EnvVar{
"Authorization": *schemas.NewEnvVar("Bearer secret-token"),
Expand Down Expand Up @@ -1504,7 +1504,7 @@ func createTestProvider(t *testing.T, db *gorm.DB, name string) uint {
func trySetupPostgresDB(t *testing.T) *gorm.DB {
t.Helper()
db, err := gorm.Open(postgres.Open(postgresDSN), &gorm.Config{
Logger: logger.Default.LogMode(logger.Silent),
Logger: gormLogger.Default.LogMode(gormLogger.Silent),
})
if err != nil {
return nil
Expand Down
10 changes: 10 additions & 0 deletions framework/configstore/tables/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package tables

import "github.com/maximhq/bifrost/core/schemas"

var logger schemas.Logger

// SetLogger sets the logger for the tables package.
func SetLogger(l schemas.Logger) {
logger = l
Comment on lines +5 to +9
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot Mar 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "tables package logger initialization:"
sed -n '1,40p' framework/configstore/tables/init.go

echo
echo "tables package logger call sites:"
rg -n 'logger\.(Warn|Info|Debug|Error|Fatal)\(' framework/configstore/tables -g '*.go'

Repository: maximhq/bifrost

Length of output: 1008

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Context around virtualkey.go logger calls (line 89) ==="
sed -n '85,95p' framework/configstore/tables/virtualkey.go

echo
echo "=== Context around virtualkey.go logger calls (line 231) ==="
sed -n '225,235p' framework/configstore/tables/virtualkey.go

echo
echo "=== Check for nil guards before logger usage ==="
rg -B5 'logger\.Warn' framework/configstore/tables/virtualkey.go | head -30

echo
echo "=== Search for SetLogger call sites ==="
rg -n 'SetLogger' --type go

echo
echo "=== Check if logger interface has a noop/nil-safe implementation ==="
rg -n 'type Logger' core/schemas -A10 --type go

Repository: maximhq/bifrost

Length of output: 6880

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Full Logger interface definition ==="
sed -n '28,60p' core/schemas/logger.go

echo
echo "=== Search for noop or default logger implementations ==="
rg -n 'type.*Logger' core/schemas --type go -A3 | grep -A3 'struct'

echo
echo "=== Check where virtualkey.go is used (imported/loaded) ==="
rg -n 'virtualkey' --type go | head -20

echo
echo "=== Check for any config loading outside main.go ==="
rg -n 'LoadConfig\|configstore\|tables' core/bifrost.go --type go -B2 -A2

echo
echo "=== Check if there are other entry points or transports ==="
fd -t f 'main.go' --type go | grep -v test

Repository: maximhq/bifrost

Length of output: 1501

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check if there are other entry points or transports ==="
find . -name 'main.go' -type f | grep -v test | grep -v '.git'

echo
echo "=== Where is virtualkey.go imported from? ==="
rg -n 'import.*virtualkey\|from.*virtualkey' --type go

echo
echo "=== Who imports tables package? ==="
rg -n 'configstore/tables' --type go | grep import

echo
echo "=== Check core/bifrost.go around line 183 (providerUtils.SetLogger) ==="
sed -n '175,195p' core/bifrost.go

echo
echo "=== Check core/bifrost.go for tables initialization/usage ==="
rg -n 'tables\.' core/bifrost.go --type go -B2 -A2

Repository: maximhq/bifrost

Length of output: 913

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Who imports tables package? ==="
rg -n 'configstore/tables' --type go | grep -E '(import|from)' | head -20

echo
echo "=== Check core/bifrost.go for tables or configstore usage ==="
rg -n 'tables\|configstore' core/bifrost.go --type go -B1 -A1

echo
echo "=== Check cli/main.go for logger and config initialization ==="
sed -n '1,80p' cli/main.go

echo
echo "=== Check if there are integration tests that load configs before SetLogger ==="
rg -n 'tables\.' --type go | grep -E 'test|_test' | head -10

Repository: maximhq/bifrost

Length of output: 245

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Find where virtualkey.go is actually used/called ==="
rg -n 'VirtualKey\|UnmarshalJSON' framework/configstore/tables --type go | head -20

echo
echo "=== Check what loads config and calls virtualkey unmarshal ==="
rg -n 'json.Unmarshal\|json.Decoder' framework/configstore --type go -B2 -A2

echo
echo "=== Check if there's an init() function in tables package that might initialize logger ==="
rg -n 'func init' framework/configstore/tables --type go

echo
echo "=== Full search for where UnmarshalJSON is triggered ==="
rg -n 'UnmarshalJSON' framework/configstore/tables -A15 --type go | head -40

echo
echo "=== Check if virtualkey package is only used during HTTP server startup ==="
grep -r 'virtualkey\|tables\.' transports/bifrost-http --include='*.go' | grep -v test | head -20

Repository: maximhq/bifrost

Length of output: 115

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== List all files in framework/configstore/tables ==="
ls -la framework/configstore/tables/

echo
echo "=== Check content of virtualkey.go ==="
head -100 framework/configstore/tables/virtualkey.go

echo
echo "=== Search for json.Unmarshal calls in framework/configstore ==="
rg -n 'json\.' framework/configstore --type go -A3 | head -50

echo
echo "=== Check how tables package is initialized in transports/bifrost-http/main.go ==="
sed -n '140,160p' transports/bifrost-http/main.go

Repository: maximhq/bifrost

Length of output: 10559

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check for any nil checks before logger.Warn calls in virtualkey.go ==="
sed -n '70,100p' framework/configstore/tables/virtualkey.go

echo
echo "=== Check around line 231 as well ==="
sed -n '215,245p' framework/configstore/tables/virtualkey.go

echo
echo "=== Check all files in CLI that might load configs ==="
head -50 cli/main.go

echo
echo "=== Check if logger interface could be nil-safe (checking Warn signature) ==="
grep -A3 'Warn(' core/schemas/logger.go

Repository: maximhq/bifrost

Length of output: 3996

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check if CLI loads/unmarshals configs before bootstrap ==="
rg -n 'config\|Config\|Unmarshal' cli/internal/app --type go -B2 -A2 | head -100

echo
echo "=== Check if any other main binaries (oauth2, plugins) set logger ==="
for f in framework/oauth2/main.go plugins/*/main.go; do
  if [ -f "$f" ]; then
    echo "=== $f ==="
    grep -n 'SetLogger\|tables\.' "$f" || echo "  (no SetLogger or tables refs)"
  fi
done

Repository: maximhq/bifrost

Length of output: 121

🏁 Script executed:

#!/bin/bash
set -euoripefail

echo "=== List files in cli/internal/app ==="
ls -la cli/internal/app/

echo
echo "=== Check main app file ==="
head -100 cli/internal/app/app.go

echo
echo "=== Search for where configstore or tables are used in CLI ==="
rg -n 'configstore\|tables' cli/ --type go

echo
echo "=== Search broadly for JSON unmarshalling in cli ==="
rg -n 'json\|Unmarshal' cli/ --type go | head -30

Repository: maximhq/bifrost

Length of output: 733

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== List files in cli/internal/app ==="
ls -la cli/internal/app/

echo
echo "=== Check main app file ==="
head -100 cli/internal/app/app.go

echo
echo "=== Search for where configstore or tables are used in CLI ==="
rg -n 'configstore\|tables' cli/ --type go

echo
echo "=== Search broadly for JSON unmarshalling in cli ==="
rg -n 'json\|Unmarshal' cli/ --type go | head -30

Repository: maximhq/bifrost

Length of output: 3072

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check all main.go files to see which ones might use configstore/tables ==="
for mainfile in $(find . -name 'main.go' -type f | grep -v test | grep -v '.git'); do
  echo "=== $mainfile ==="
  grep -l 'configstore\|tables\|SetLogger' "$mainfile" && echo "USES CONFIGSTORE/TABLES" || echo "does not use configstore"
done

echo
echo "=== Check framework/oauth2/main.go ==="
cat framework/oauth2/main.go

echo
echo "=== Check core/bifrost.go for tables/configstore usage (around line 183) ==="
sed -n '150,200p' core/bifrost.go

Repository: maximhq/bifrost

Length of output: 27179

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check plugins/governance/main.go for SetLogger call ==="
grep -n 'SetLogger\|tables\.' plugins/governance/main.go

echo
echo "=== Check plugins/logging/main.go for SetLogger call ==="
grep -n 'SetLogger\|tables\.' plugins/logging/main.go

echo
echo "=== Check framework/modelcatalog/main.go for SetLogger call ==="
grep -n 'SetLogger\|tables\.' framework/modelcatalog/main.go

echo
echo "=== Check the actual imports in these files to see what configstore they use ==="
head -50 plugins/governance/main.go | grep -A5 'import'

echo
echo "=== Check if any plugin loads virtualkey configs during init/main ==="
rg -n 'virtual.*key\|Virtual.*Key' plugins/ framework/modelcatalog/ --type go | head -20

Repository: maximhq/bifrost

Length of output: 118

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== List what's in plugins/governance ==="
ls -la plugins/governance/

echo
echo "=== Check plugins/governance/main.go ==="
cat plugins/governance/main.go

echo
echo "=== Check plugins/logging/main.go ==="
cat plugins/logging/main.go

Repository: maximhq/bifrost

Length of output: 50372

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check if governance plugin calls tables.SetLogger ==="
grep -n 'tables\.SetLogger\|configstoreTables\.SetLogger' plugins/governance/main.go

echo
echo "=== Check framework/modelcatalog/main.go ==="
head -100 framework/modelcatalog/main.go

echo
echo "=== Check if modelcatalog calls tables.SetLogger ==="
grep -n 'tables\.SetLogger\|SetLogger' framework/modelcatalog/main.go

echo
echo "=== Search for all SetLogger calls to tables package across the codebase ==="
rg -n 'tables\.SetLogger' --type go

Repository: maximhq/bifrost

Length of output: 116

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== ALL places where tables.SetLogger is actually called ==="
rg -n 'tables\.SetLogger' --type go

echo
echo "=== Check if governance plugin loads virtual key configs in Init ==="
grep -n 'VirtualKey\|UnmarshalJSON\|configstore' plugins/governance/main.go | head -20

echo
echo "=== Check what tables are loaded by governance in NewLocalGovernanceStore ==="
sed -n '1,100p' plugins/governance/store.go | grep -A10 'NewLocalGovernanceStore\|configStore'

echo
echo "=== Confirm: does governance plugin's store load virtual keys? ==="
rg -n 'GetVirtualKey\|TableVirtualKey' plugins/governance/store.go --type go | head -10

Repository: maximhq/bifrost

Length of output: 1824

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== Check transports/bifrost-http/main.go initialization order ==="
sed -n '130,160p' transports/bifrost-http/main.go

echo
echo "=== Check where governance plugin Init is called in the HTTP transport ==="
rg -n 'governance\.Init\|NewGovernancePlugin' transports/bifrost-http --type go -B3 -A3

echo
echo "=== Check server.go or server initialization for governance setup ==="
grep -n 'governance\|GovernancePlugin' transports/bifrost-http/server/server.go | head -20

Repository: maximhq/bifrost

Length of output: 1167

Initialize the tables logger to a safe default or add nil checks before logging calls.

logger is an uninitialized package-global variable. The UnmarshalJSON methods in virtualkey.go (lines 89, 231) call logger.Warn() unconditionally without nil guards. Any code that unmarshals virtual key configs before tables.SetLogger() runs will panic on a nil interface method call. While the HTTP transport calls tables.SetLogger() before bootstrap, other entry points (governance plugin, oauth2 framework, modelcatalog) may load configs without wiring the logger. Initialize this package logger to a no-op default implementation, or guard all logger calls with nil checks.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@framework/configstore/tables/init.go` around lines 5 - 9, The package-global
logger variable is nil and UnmarshalJSON in virtualkey.go calls logger.Warn()
unguarded; initialize logger to a safe no-op default that implements
schemas.Logger (so logging calls are safe before SetLogger) and keep SetLogger(l
schemas.Logger) to replace that default; alternatively, add nil checks around
logger usage in UnmarshalJSON (functions named UnmarshalJSON at virtualkey.go
lines ~89 and ~231) to call logger only if non-nil. Ensure the no-op implements
the same methods as schemas.Logger so all existing logger.Warn/info/error calls
remain valid.

}
85 changes: 69 additions & 16 deletions framework/configstore/tables/virtualkey.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,33 +44,65 @@ func (TableVirtualKeyProviderConfig) TableName() string {
return "governance_virtual_key_provider_configs"
}

// UnmarshalJSON custom unmarshaller to handle both "keys" ([]TableKey) and "allowed_keys" ([]string) formats
// UnmarshalJSON custom unmarshaller to handle both "keys" ([]TableKey) and "allowed_keys" ([]string) formats.
//
// Two formats are supported:
// - Config file format: "allowed_keys" field ([]string or absent)
// - DB/API format: "allow_all_keys" bool + "keys" []TableKey
//
// Absent vs empty semantics for config file format:
// - allowed_keys absent → AllowAllKeys = true (allow all by default)
// - allowed_keys: [] → AllowAllKeys = false (deny all — explicit empty)
// - allowed_keys: ["*"] → AllowAllKeys = true (explicit allow-all wildcard)
// - allowed_keys: ["k1","k2"] → specific keys, AllowAllKeys = false
func (pc *TableVirtualKeyProviderConfig) UnmarshalJSON(data []byte) error {
// Temporary struct to capture all fields including allowed_keys
// Temporary struct to capture all fields.
// - AllowedKeys uses *[]string so we can distinguish absent (nil) from empty ([]).
// - DBAllowAllKeys uses *bool to detect whether "allow_all_keys" is present in JSON
// (DB/API format), shadowing the bool in Alias so we can tell config vs DB format.
type Alias TableVirtualKeyProviderConfig
type TempProviderConfig struct {
Alias
AllowedKeys []string `json:"allowed_keys"` // Config file format: array of key names
AllowedKeys *[]string `json:"allowed_keys"` // Config file format; nil = absent
DBAllowAllKeys *bool `json:"allow_all_keys"` // DB/API format; nil = absent (shadows Alias field)
}

var temp TempProviderConfig
if err := json.Unmarshal(data, &temp); err != nil {
return err
}

// Copy all standard fields
// Copy all standard fields (AllowAllKeys in Alias will be false because DBAllowAllKeys shadows
// the "allow_all_keys" JSON key; we restore the correct value below).
*pc = TableVirtualKeyProviderConfig(temp.Alias)

// If allowed_keys is provided (config file format), convert to Keys or set AllowAllKeys
// This takes precedence if Keys is empty but allowed_keys has values
if len(temp.AllowedKeys) > 0 && len(pc.Keys) == 0 {
// Check for wildcard — ["*"] means allow all keys
if len(temp.AllowedKeys) == 1 && temp.AllowedKeys[0] == "*" {
if temp.DBAllowAllKeys != nil {
// DB/API format: "allow_all_keys" was explicitly present — use it directly.
pc.AllowAllKeys = *temp.DBAllowAllKeys
} else if len(pc.Keys) == 0 {
// Config file format (no "allow_all_keys" JSON field).
// Apply absent-vs-empty semantics for allowed_keys.
if temp.AllowedKeys == nil {
// absent → allow all by default.
// DEPRECATED (next major version): absent allowed_keys will mean deny-all, same as [].
// Add allowed_keys: ["*"] or list specific keys explicitly in your config.json.
logger.Warn("[DEPRECATED] virtual key provider config for provider %q has no allowed_keys in config.json — defaulting to allow all keys. This implicit allow-all will be removed in the next major version. Use allowed_keys: [\"*\"] to allow all keys explicitly, or list specific key names.",
pc.Provider)
pc.AllowAllKeys = true
} else {
pc.Keys = make([]TableKey, len(temp.AllowedKeys))
for i, keyName := range temp.AllowedKeys {
pc.Keys[i] = TableKey{Name: keyName}
keys := *temp.AllowedKeys
if len(keys) == 0 {
// explicit empty [] → deny all (AllowAllKeys = false, Keys = [])
pc.AllowAllKeys = false
} else if len(keys) == 1 && keys[0] == "*" {
// wildcard ["*"] → allow all
pc.AllowAllKeys = true
} else {
// specific key names
pc.Keys = make([]TableKey, len(keys))
for i, keyName := range keys {
pc.Keys[i] = TableKey{Name: keyName}
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
Expand Down Expand Up @@ -166,22 +198,43 @@ func (TableVirtualKeyMCPConfig) TableName() string {

// UnmarshalJSON custom unmarshaller to handle both "mcp_client_id" (database format)
// and "mcp_client_name" (config file format) for MCP client references.
//
// Absent vs empty semantics for tools_to_execute in config file format:
// - tools_to_execute absent → ["*"] (allow all tools for this client by default)
// - tools_to_execute: [] → [] (deny all tools for this client — explicit empty)
// - tools_to_execute: ["*"] → ["*"] (explicit allow-all wildcard)
// - tools_to_execute: ["t"] → ["t"] (specific tools only)
func (mc *TableVirtualKeyMCPConfig) UnmarshalJSON(data []byte) error {
// Temporary struct to capture all fields including mcp_client_name
// Temporary struct to capture all fields.
// ToolsToExecute uses *[]string to distinguish absent (nil) from empty ([]).
// The outer *[]string shadows the []string on Alias for JSON unmarshalling.
type Alias TableVirtualKeyMCPConfig
type TempMCPConfig struct {
Alias
MCPClientName string `json:"mcp_client_name"` // Config file format: MCP client name
MCPClientName string `json:"mcp_client_name"` // Config file format: MCP client name
ToolsToExecute *[]string `json:"tools_to_execute"` // pointer: nil = absent, non-nil = present
}

var temp TempMCPConfig
if err := json.Unmarshal(data, &temp); err != nil {
return err
}

// Copy all standard fields
// Copy all standard fields (Alias.ToolsToExecute is nil because the outer *[]string shadows it)
*mc = TableVirtualKeyMCPConfig(temp.Alias)

// Apply absent-vs-empty semantics for tools_to_execute.
// DEPRECATED (next major version): absent tools_to_execute will mean deny-all, same as [].
// Add tools_to_execute: ["*"] to allow all tools explicitly, or list specific tool names.
if temp.ToolsToExecute == nil {
// absent → allow all tools for this client by default.
logger.Warn("[DEPRECATED] virtual key MCP config for client %q has no tools_to_execute in config.json — defaulting to allow all tools. This implicit allow-all will be removed in the next major version. Use tools_to_execute: [\"*\"] to allow all tools explicitly, or list specific tool names.",
temp.MCPClientName)
mc.ToolsToExecute = []string{"*"}
} else {
mc.ToolsToExecute = *temp.ToolsToExecute
}

// Capture mcp_client_name for later resolution to MCPClientID
if temp.MCPClientName != "" {
mc.MCPClientName = temp.MCPClientName
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Expand All @@ -198,7 +251,7 @@ type TableVirtualKey struct {
Value string `gorm:"uniqueIndex:idx_virtual_key_value;type:text;not null" json:"value"` // The virtual key value
IsActive bool `gorm:"default:true" json:"is_active"`
ProviderConfigs []TableVirtualKeyProviderConfig `gorm:"foreignKey:VirtualKeyID;constraint:OnDelete:CASCADE" json:"provider_configs"` // Empty means no providers allowed (deny-by-default)
MCPConfigs []TableVirtualKeyMCPConfig `gorm:"foreignKey:VirtualKeyID;constraint:OnDelete:CASCADE" json:"mcp_configs"`
MCPConfigs []TableVirtualKeyMCPConfig `gorm:"foreignKey:VirtualKeyID;constraint:OnDelete:CASCADE" json:"mcp_configs"` // Empty means no MCP clients allowed (deny-by-default)

// Foreign key relationships (mutually exclusive: either TeamID or CustomerID, not both)
TeamID *string `gorm:"type:varchar(255);index" json:"team_id,omitempty"`
Expand Down
73 changes: 67 additions & 6 deletions transports/bifrost-http/lib/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -961,18 +961,84 @@ func loadGovernanceConfigFromFile(ctx context.Context, config *Config, configDat
config.GovernanceConfig = governanceConfig
// Merge with config file if present
if configData.Governance != nil {
preprocessGovernanceVirtualKeys(ctx, config, configData)
mergeGovernanceConfig(ctx, config, configData, governanceConfig)
}
} else if configData.Governance != nil {
// No governance config in store, use config file
logger.Debug("no governance config found in store, processing from config file")
preprocessGovernanceVirtualKeys(ctx, config, configData)
config.GovernanceConfig = configData.Governance
createGovernanceConfigInStore(ctx, config)
} else {
logger.Debug("no governance config in store or config file")
}
}

// preprocessGovernanceVirtualKeys expands absent provider_configs / mcp_configs on each VK
// in configData to all currently configured providers / MCP clients.
//
// Go's JSON decoder sets a slice field to nil when the key is absent, and to a non-nil
// empty slice when the key is present but empty ([]). We use this to distinguish:
// - absent → allow all (expand to every provider / MCP client present at init time)
// - explicit [] → deny all (leave as empty slice; resolver enforces deny-by-default)
//
// DEPRECATED (next major version): relying on an absent key for allow-all behaviour will be
// removed. Always include the field explicitly in your config.json:
// - to allow all providers: list them explicitly in provider_configs
// - to deny all providers: set provider_configs: []
// - to allow all MCP clients: list them explicitly in mcp_configs
// - to deny all MCP clients: set mcp_configs: []
// In the next major version both absent and [] will mean deny-all (deny-by-default).
func preprocessGovernanceVirtualKeys(ctx context.Context, config *Config, configData *ConfigData) {
for i := range configData.Governance.VirtualKeys {
vk := &configData.Governance.VirtualKeys[i]

// Absent provider_configs → allow all providers configured right now.
// DEPRECATED: explicitly list provider_configs in config.json instead of relying on absence.
if vk.ProviderConfigs == nil {
logger.Warn(
"virtual key %q has no provider_configs in config.json — expanding to all %d configured provider(s). "+
"This implicit allow-all behaviour is DEPRECATED and will be removed in the next major version. "+
"Please add an explicit provider_configs list to your config.json.",
vk.ID, len(config.Providers),
)
expanded := make([]configstoreTables.TableVirtualKeyProviderConfig, 0, len(config.Providers))
for providerName := range config.Providers {
expanded = append(expanded, configstoreTables.TableVirtualKeyProviderConfig{
Provider: string(providerName),
AllowAllKeys: true,
})
}
vk.ProviderConfigs = expanded
}

// Absent mcp_configs → allow all MCP clients configured right now.
// DEPRECATED: explicitly list mcp_configs in config.json instead of relying on absence.
if vk.MCPConfigs == nil && config.MCPConfig != nil && len(config.MCPConfig.ClientConfigs) > 0 {
logger.Warn(
"virtual key %q has no mcp_configs in config.json — expanding to all %d configured MCP client(s). "+
"This implicit allow-all behaviour is DEPRECATED and will be removed in the next major version. "+
"Please add an explicit mcp_configs list to your config.json.",
vk.ID, len(config.MCPConfig.ClientConfigs),
)
vk.MCPConfigs = make([]configstoreTables.TableVirtualKeyMCPConfig, 0, len(config.MCPConfig.ClientConfigs))
for _, client := range config.MCPConfig.ClientConfigs {
vk.MCPConfigs = append(vk.MCPConfigs, configstoreTables.TableVirtualKeyMCPConfig{
MCPClientName: client.Name,
ToolsToExecute: []string{"*"},
})
}
}

// Resolve client names → DB IDs for all MCPConfigs (including explicitly-listed ones)
// so that MCPClientID is populated before GenerateVirtualKeyHash is called.
if len(vk.MCPConfigs) > 0 {
vk.MCPConfigs = resolveMCPConfigClientIDs(ctx, config.ConfigStore, vk.MCPConfigs, vk.ID)
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}
}

// mergeGovernanceConfig merges governance config from file with store
func mergeGovernanceConfig(ctx context.Context, config *Config, configData *ConfigData, governanceConfig *configstore.GovernanceConfig) {
logger.Debug("merging governance config from config file with store")
Expand Down Expand Up @@ -1139,9 +1205,6 @@ func mergeGovernanceConfig(ctx context.Context, config *Config, configData *Conf
}
configData.Governance.VirtualKeys[i].Value = governance.GenerateVirtualKey()
}
// Resolve MCP client names to IDs for config file mcp_configs
configData.Governance.VirtualKeys[i].MCPConfigs = resolveMCPConfigClientIDs(
ctx, config.ConfigStore, configData.Governance.VirtualKeys[i].MCPConfigs, newVirtualKey.ID)
virtualKeysToUpdate = append(virtualKeysToUpdate, configData.Governance.VirtualKeys[i])
governanceConfig.VirtualKeys[j] = configData.Governance.VirtualKeys[i]
} else {
Expand Down Expand Up @@ -1169,9 +1232,6 @@ func mergeGovernanceConfig(ctx context.Context, config *Config, configData *Conf
}
configData.Governance.VirtualKeys[i].Value = governance.GenerateVirtualKey()
}
// Resolve MCP client names to IDs for config file mcp_configs
configData.Governance.VirtualKeys[i].MCPConfigs = resolveMCPConfigClientIDs(
ctx, config.ConfigStore, configData.Governance.VirtualKeys[i].MCPConfigs, newVirtualKey.ID)
virtualKeysToAdd = append(virtualKeysToAdd, configData.Governance.VirtualKeys[i])
}
}
Expand Down Expand Up @@ -2386,6 +2446,7 @@ func reconcileVirtualKeyAssociations(
// Update existing provider config from file
existing.Weight = newPC.Weight
existing.AllowedModels = newPC.AllowedModels
existing.AllowAllKeys = newPC.AllowAllKeys
existing.BudgetID = newPC.BudgetID
existing.RateLimitID = newPC.RateLimitID
existing.Keys = newPC.Keys
Expand Down
2 changes: 2 additions & 0 deletions transports/bifrost-http/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import (

bifrost "github.com/maximhq/bifrost/core"
schemas "github.com/maximhq/bifrost/core/schemas"
"github.com/maximhq/bifrost/framework/configstore/tables"
"github.com/maximhq/bifrost/transports/bifrost-http/handlers"
"github.com/maximhq/bifrost/transports/bifrost-http/lib"
bifrostServer "github.com/maximhq/bifrost/transports/bifrost-http/server"
Expand Down Expand Up @@ -143,6 +144,7 @@ func main() {
lib.SetLogger(logger)
bifrostServer.SetLogger(logger)
handlers.SetLogger(logger)
tables.SetLogger(logger)

ctx := context.Background()
err := server.Bootstrap(ctx)
Expand Down
27 changes: 27 additions & 0 deletions transports/changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,30 @@
- feat: VK provider config key_ids now supports ["*"] wildcard to allow all keys; empty key_ids denies all; handler resolves wildcard to AllowAllKeys flag without DB key lookups
- feat: add option to disable automatic MCP tool injection per request
- feat: virtual key MCP configs now act as an execution-time allow-list — tools not permitted by the VK are blocked at inference and MCP tool execution

## BREAKING CHANGES — explicit empty arrays now mean deny-all; absent inner fields now mean allow-all

The following fields in `governance.virtual_keys[*]` in config.json have changed semantics:

| Field | Old behaviour | New behaviour | Type |
|---|---|---|---|
| `provider_configs: []` | allow all providers | **deny all providers** | breaking |
| `provider_configs` absent | allow all providers | allow all providers — deprecated, see below | deprecated |
| `provider_configs[*].allowed_keys: []` | allow all keys | **deny all keys** | breaking |
| `provider_configs[*].allowed_keys` absent | deny all keys (bug) | **allow all keys** — deprecated, see below | breaking + deprecated |
| `mcp_configs: []` | allow all MCP clients | **deny all MCP clients** | breaking |
| `mcp_configs` absent | allow all MCP clients | allow all MCP clients — deprecated, see below | deprecated |
| `mcp_configs[*].tools_to_execute: []` | deny all tools | deny all tools | unchanged |
| `mcp_configs[*].tools_to_execute` absent | deny all tools (bug) | **allow all tools** — deprecated, see below | breaking + deprecated |

### Migration guide

**`provider_configs: []` / `mcp_configs: []`** — previously allowed all; now deny all. To keep allow-all behaviour, list entries explicitly instead of using an empty array.

**`provider_configs[*].allowed_keys` absent** — previously (buggy) denied all keys; now allows all keys and emits a deprecation warning. If you intended deny-all, add `allowed_keys: []` explicitly. If you intended allow-all, add `allowed_keys: ["*"]` to silence the warning.

**`mcp_configs[*].tools_to_execute` absent** — previously (buggy) denied all tools for that client; now allows all tools and emits a deprecation warning. If you intended deny-all, add `tools_to_execute: []` explicitly. If you intended allow-all, add `tools_to_execute: ["*"]` to silence the warning.

### Deprecation notice

Omitting `provider_configs`, `mcp_configs`, `allowed_keys`, or `tools_to_execute` entirely (absent key) currently defaults to allow-all and emits a startup warning. For `provider_configs` and `mcp_configs`, the implicit allow-all expands to whichever providers / MCP clients are present at startup — it is a boot-time snapshot, not a live wildcard. **This implicit allow-all will be removed in the next major version** — absent and `[]` will both mean deny-all (deny-by-default). Migrate by always specifying these fields explicitly in config.json.
2 changes: 1 addition & 1 deletion ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ export default function VirtualKeySheet({ virtualKey, teams, customers, onSave,
{
label: "Allow All Tools",
value: "*",
description: "Allow all current and future tools (including dynamically fetched ones)",
description: "Allow all current and future tools",
},
...[...availableTools, ...enabledToolsByConfig]
.filter((tool, index, arr) => arr.findIndex((t) => t.name === tool.name) === index)
Expand Down