refactor: standardize empty array conventions for VK Provider Config Allowed Keys

core/utils.go

@@ -193,6 +193,7 @@ func newBifrostMessageChan(message *schemas.BifrostResponse) chan *schemas.Bifro
 func clearCtxForFallback(ctx *schemas.BifrostContext) {
 	ctx.ClearValue(schemas.BifrostContextKeyAPIKeyID)
 	ctx.ClearValue(schemas.BifrostContextKeyAPIKeyName)
+	ctx.ClearValue(schemas.BifrostContextKeyGovernanceIncludeOnlyKeys)
 }
 
 var supportedBaseProvidersSet = func() map[schemas.ModelProvider]struct{} {

framework/changelog.md

@@ -1 +1,2 @@
+- feat: migrate VK provider config allowed keys to explicit allow-list semantics — add AllowAllKeys bool to TableVirtualKeyProviderConfig; backfill existing configs with allow_all_keys=true; empty keys now denies all, ["*"] allows all
 - feat: add MCPDisableAutoToolInject column to TableClientConfig

framework/configstore/migrations.go

@@ -320,6 +320,9 @@ func triggerMigrations(ctx context.Context, db *gorm.DB) error {
 	if err := migrationAddMCPDisableAutoToolInjectColumn(ctx, db); err != nil {
 		return err
 	}
+	if err := migrationAddAllowAllKeysToProviderConfig(ctx, db); err != nil {
+		return err
+	}
 	return nil
 }
 
@@ -4338,6 +4341,94 @@ func migrationAddBedrockAssumeRoleColumns(ctx context.Context, db *gorm.DB) erro
 	return nil
 }
 
+// migrationAddAllowAllKeysToProviderConfig adds the allow_all_keys column to the provider config table
+// and backfills existing rows: any provider config with no keys in the join table previously meant
+// "allow all keys" (old semantic), so they get allow_all_keys = true to preserve behaviour.
+func migrationAddAllowAllKeysToProviderConfig(ctx context.Context, db *gorm.DB) error {
+	m := migrator.New(db, migrator.DefaultOptions, []*migrator.Migration{{
+		ID: "add_allow_all_keys_to_provider_config",
+		Migrate: func(tx *gorm.DB) error {
+			tx = tx.WithContext(ctx)
+			migratorInstance := tx.Migrator()
+
+			// Add the column if it doesn't exist
+			if !migratorInstance.HasColumn(&tables.TableVirtualKeyProviderConfig{}, "allow_all_keys") {
+				if err := migratorInstance.AddColumn(&tables.TableVirtualKeyProviderConfig{}, "allow_all_keys"); err != nil {
+					return fmt.Errorf("failed to add allow_all_keys column: %w", err)
+				}
+			}
+
+			// Backfill: find all provider configs that have no keys in the join table.
+			// These previously meant "allow all keys", so set allow_all_keys = true.
+			var allConfigs []tables.TableVirtualKeyProviderConfig
+			if err := tx.Find(&allConfigs).Error; err != nil {
+				return fmt.Errorf("failed to query provider configs: %w", err)
+			}
+
+			// Track which VK IDs were modified so we can recompute their config_hash.
+			// Without this, subsequent config-sync diff logic would see a stale hash
+			// and attempt to re-reconcile the VK (potentially undoing the backfill).
+			modifiedVKIDs := make(map[string]struct{})
+
+			for _, pc := range allConfigs {
+				var keyCount int64
+				if err := tx.Table("governance_virtual_key_provider_config_keys").
+					Where("table_virtual_key_provider_config_id = ?", pc.ID).
+					Count(&keyCount).Error; err != nil {
+					return fmt.Errorf("failed to count keys for provider config %d: %w", pc.ID, err)
+				}
+
+				if keyCount == 0 {
+					if err := tx.Model(&tables.TableVirtualKeyProviderConfig{}).
+						Where("id = ?", pc.ID).
+						Update("allow_all_keys", true).Error; err != nil {
+						return fmt.Errorf("failed to backfill allow_all_keys for provider config %d: %w", pc.ID, err)
+					}
+					modifiedVKIDs[pc.VirtualKeyID] = struct{}{}
+				}
+			}
+
+			// Recompute and persist config_hash for every VK that was modified.
+			for vkID := range modifiedVKIDs {
+				var vk tables.TableVirtualKey
+				if err := tx.
+					Preload("ProviderConfigs").
+					Preload("ProviderConfigs.Keys").
+					Preload("MCPConfigs").
+					First(&vk, "id = ?", vkID).Error; err != nil {
+					return fmt.Errorf("failed to reload VK %s for hash recomputation: %w", vkID, err)
+				}
+				newHash, err := GenerateVirtualKeyHash(vk)
+				if err != nil {
+					return fmt.Errorf("failed to generate hash for VK %s: %w", vkID, err)
+				}
+				if err := tx.Model(&tables.TableVirtualKey{}).
+					Where("id = ?", vkID).
+					Update("config_hash", newHash).Error; err != nil {
+					return fmt.Errorf("failed to update config_hash for VK %s: %w", vkID, err)
+				}
+				log.Printf("[Migration] Recomputed config_hash for VK '%s'", vk.Name)
+			}
+
+			return nil
+		},
+		Rollback: func(tx *gorm.DB) error {
+			tx = tx.WithContext(ctx)
+			migratorInstance := tx.Migrator()
+			if migratorInstance.HasColumn(&tables.TableVirtualKeyProviderConfig{}, "allow_all_keys") {
+				if err := migratorInstance.DropColumn(&tables.TableVirtualKeyProviderConfig{}, "allow_all_keys"); err != nil {
+					return fmt.Errorf("failed to drop allow_all_keys column: %w", err)
+				}
+			}
+			return nil
+		},
+	}})
+	if err := m.Migrate(); err != nil {
+		return fmt.Errorf("error running allow_all_keys migration: %s", err.Error())
+	}
+	return nil
+}
+
 // migrationAddMCPDisableAutoToolInjectColumn adds the mcp_disable_auto_tool_inject column to the client config table.
 // When true, MCP tools are not automatically injected into requests; only explicit context filters apply.
 func migrationAddMCPDisableAutoToolInjectColumn(ctx context.Context, db *gorm.DB) error {

framework/configstore/tables/virtualkey.go

@@ -29,13 +29,14 @@ type TableVirtualKeyProviderConfig struct {
 	Provider      string   `gorm:"type:varchar(50);not null" json:"provider"`
 	Weight        *float64 `json:"weight"`
 	AllowedModels []string `gorm:"type:text;serializer:json" json:"allowed_models"` // Empty means all models allowed
+	AllowAllKeys  bool     `gorm:"default:false" json:"allow_all_keys"`             // True means all keys allowed; false with empty Keys means no keys allowed (deny-by-default)
 	BudgetID      *string  `gorm:"type:varchar(255);index" json:"budget_id,omitempty"`
 	RateLimitID   *string  `gorm:"type:varchar(255);index" json:"rate_limit_id,omitempty"`
 
 	// Relationships
 	Budget    *TableBudget    `gorm:"foreignKey:BudgetID;onDelete:CASCADE" json:"budget,omitempty"`
 	RateLimit *TableRateLimit `gorm:"foreignKey:RateLimitID;onDelete:CASCADE" json:"rate_limit,omitempty"`
-	Keys      []TableKey      `gorm:"many2many:governance_virtual_key_provider_config_keys;constraint:OnDelete:CASCADE" json:"keys"` // Empty means all keys allowed for this provider
+	Keys      []TableKey      `gorm:"many2many:governance_virtual_key_provider_config_keys;constraint:OnDelete:CASCADE" json:"keys"` // Used when AllowAllKeys is false; empty means no keys allowed
 }
 
 // TableName sets the table name for each model
@@ -60,12 +61,17 @@ func (pc *TableVirtualKeyProviderConfig) UnmarshalJSON(data []byte) error {
 	// Copy all standard fields
 	*pc = TableVirtualKeyProviderConfig(temp.Alias)
 
-	// If allowed_keys is provided (config file format), convert to Keys
+	// 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 {
-		pc.Keys = make([]TableKey, len(temp.AllowedKeys))
-		for i, keyName := range temp.AllowedKeys {
-			pc.Keys[i] = TableKey{Name: keyName}
+		// Check for wildcard — ["*"] means allow all keys
+		if len(temp.AllowedKeys) == 1 && temp.AllowedKeys[0] == "*" {
+			pc.AllowAllKeys = true
+		} else {
+			pc.Keys = make([]TableKey, len(temp.AllowedKeys))
+			for i, keyName := range temp.AllowedKeys {
+				pc.Keys[i] = TableKey{Name: keyName}
+			}
 		}
 	}
 

plugins/governance/changelog.md

@@ -1 +1,2 @@
+- feat: migrate VK provider config allowed keys to deny-by-default — `key_ids: ["*"]` (API) / `allowed_keys: ["*"]` (config) maps to `AllowAllKeys=true` without DB lookups; empty list sets `AllowAllKeys=false` and blocks all keys; resolver populates `includeOnlyKeys` for specific keys or clears it to nil for allow-all to prevent stale filters
 - feat: enforce VK MCPConfigs as an execution-time allow-list — empty MCPConfigs denies all MCP tools, non-empty validates each tool in both PreMCPHook and evaluateGovernanceRequest; respects disable_auto_tool_inject toggle (transport config key: mcp_disable_auto_tool_inject) and skips auto-injection header when caller already set it

plugins/governance/main.go

@@ -1012,10 +1012,10 @@ func (p *GovernancePlugin) evaluateGovernanceRequest(ctx *schemas.BifrostContext
 	if result.Decision == DecisionAllow && evaluationRequest.VirtualKey != "" {
 		if evaluationRequest.UserID != "" {
 			// User auth present: only use VK for routing/filtering (skip rate limits and budgets)
-			result = p.resolver.EvaluateVirtualKeyFiltering(ctx, evaluationRequest.VirtualKey, evaluationRequest.Provider, evaluationRequest.Model, requestType)
+			result = p.resolver.EvaluateVirtualKeyRequest(ctx, evaluationRequest.VirtualKey, evaluationRequest.Provider, evaluationRequest.Model, requestType, true)
 		} else {
 			// No user auth: full VK governance (routing + limits)
-			result = p.resolver.EvaluateVirtualKeyRequest(ctx, evaluationRequest.VirtualKey, evaluationRequest.Provider, evaluationRequest.Model, requestType)
+			result = p.resolver.EvaluateVirtualKeyRequest(ctx, evaluationRequest.VirtualKey, evaluationRequest.Provider, evaluationRequest.Model, requestType, false)
 		}
 	}
 

plugins/governance/resolver.go

@@ -171,7 +171,8 @@ func (r *BudgetResolver) isModelRequired(requestType schemas.RequestType) bool {
 }
 
 // EvaluateVirtualKeyRequest evaluates virtual key-specific checks including validation, filtering, rate limits, and budgets
-func (r *BudgetResolver) EvaluateVirtualKeyRequest(ctx *schemas.BifrostContext, virtualKeyValue string, provider schemas.ModelProvider, model string, requestType schemas.RequestType) *EvaluationResult {
+// skipRateLimitsAndBudgets evaluates to true when we want to skip rate limits and budgets. This is used when user auth is present (user governance handles limits).
+func (r *BudgetResolver) EvaluateVirtualKeyRequest(ctx *schemas.BifrostContext, virtualKeyValue string, provider schemas.ModelProvider, model string, requestType schemas.RequestType, skipRateLimitsAndBudgets bool) *EvaluationResult {
 	// 1. Validate virtual key exists and is active
 	vk, exists := r.store.GetVirtualKey(virtualKeyValue)
 	if !exists {
@@ -225,100 +226,36 @@ func (r *BudgetResolver) EvaluateVirtualKeyRequest(ctx *schemas.BifrostContext,
 	}
 
 	// 4. Check rate limits hierarchy (VK level)
-	if rateLimitResult := r.checkRateLimitHierarchy(ctx, vk, evaluationRequest); rateLimitResult != nil {
-		return rateLimitResult
-	}
-
-	// 5. Check budget hierarchy (VK → Team → Customer)
-	if budgetResult := r.checkBudgetHierarchy(ctx, vk, evaluationRequest); budgetResult != nil {
-		return budgetResult
-	}
-
-	// Find the provider config that matches the request's provider and get its allowed keys
-	for _, pc := range vk.ProviderConfigs {
-		if schemas.ModelProvider(pc.Provider) == provider && len(pc.Keys) > 0 {
-			includeOnlyKeys := make([]string, 0, len(pc.Keys))
-			for _, dbKey := range pc.Keys {
-				includeOnlyKeys = append(includeOnlyKeys, dbKey.KeyID)
-			}
-			ctx.SetValue(schemas.BifrostContextKeyGovernanceIncludeOnlyKeys, includeOnlyKeys)
-			break
+	if !skipRateLimitsAndBudgets {
+		if rateLimitResult := r.checkRateLimitHierarchy(ctx, vk, evaluationRequest); rateLimitResult != nil {
+			return rateLimitResult
 		}
-	}
-
-	// All checks passed
-	return &EvaluationResult{
-		Decision:   DecisionAllow,
-		Reason:     "Request allowed by governance policy",
-		VirtualKey: vk,
-	}
-}
 
-// EvaluateVirtualKeyFiltering evaluates virtual key checks for routing and model/provider filtering only,
-// skipping rate limits and budgets. Used when user auth is present (user governance handles limits).
-func (r *BudgetResolver) EvaluateVirtualKeyFiltering(ctx *schemas.BifrostContext, virtualKeyValue string, provider schemas.ModelProvider, model string, requestType schemas.RequestType) *EvaluationResult {
-	// 1. Validate virtual key exists and is active
-	vk, exists := r.store.GetVirtualKey(virtualKeyValue)
-	if !exists {
-		return &EvaluationResult{
-			Decision: DecisionVirtualKeyNotFound,
-			Reason:   "Virtual key not found",
-		}
-	}
-	// Set virtual key id and name in context
-	ctx.SetValue(schemas.BifrostContextKeyGovernanceVirtualKeyID, vk.ID)
-	ctx.SetValue(schemas.BifrostContextKeyGovernanceVirtualKeyName, vk.Name)
-	if vk.Team != nil {
-		ctx.SetValue(schemas.BifrostContextKeyGovernanceTeamID, vk.Team.ID)
-		ctx.SetValue(schemas.BifrostContextKeyGovernanceTeamName, vk.Team.Name)
-		if vk.Team.Customer != nil {
-			ctx.SetValue(schemas.BifrostContextKeyGovernanceCustomerID, vk.Team.Customer.ID)
-			ctx.SetValue(schemas.BifrostContextKeyGovernanceCustomerName, vk.Team.Customer.Name)
-		}
-	}
-	if vk.Customer != nil {
-		ctx.SetValue(schemas.BifrostContextKeyGovernanceCustomerID, vk.Customer.ID)
-		ctx.SetValue(schemas.BifrostContextKeyGovernanceCustomerName, vk.Customer.Name)
-	}
-	if !vk.IsActive {
-		return &EvaluationResult{
-			Decision: DecisionVirtualKeyBlocked,
-			Reason:   "Virtual key is inactive",
-		}
-	}
-	// 2. Check provider filtering
-	if requestType != schemas.MCPToolExecutionRequest && !r.isProviderAllowed(vk, provider) {
-		return &EvaluationResult{
-			Decision:   DecisionProviderBlocked,
-			Reason:     fmt.Sprintf("Provider '%s' is not allowed for this virtual key", provider),
-			VirtualKey: vk,
-		}
-	}
-	// 3. Check model filtering
-	if r.isModelRequired(requestType) && !r.isModelAllowed(vk, provider, model) {
-		return &EvaluationResult{
-			Decision:   DecisionModelBlocked,
-			Reason:     fmt.Sprintf("Model '%s' is not allowed for this virtual key", model),
-			VirtualKey: vk,
+		// 5. Check budget hierarchy (VK → Team → Customer)
+		if budgetResult := r.checkBudgetHierarchy(ctx, vk, evaluationRequest); budgetResult != nil {
+			return budgetResult
 		}
 	}
 
-	// Set include-only keys for provider config routing
+	// Find the provider config that matches the request's provider and apply key filtering
 	for _, pc := range vk.ProviderConfigs {
-		if schemas.ModelProvider(pc.Provider) == provider && len(pc.Keys) > 0 {
-			includeOnlyKeys := make([]string, 0, len(pc.Keys))
-			for _, dbKey := range pc.Keys {
-				includeOnlyKeys = append(includeOnlyKeys, dbKey.KeyID)
+		if schemas.ModelProvider(pc.Provider) == provider {
+			if !pc.AllowAllKeys {
+				// Restrict to specific keys (empty slice = no keys allowed)
+				includeOnlyKeys := make([]string, 0, len(pc.Keys))
+				for _, dbKey := range pc.Keys {
+					includeOnlyKeys = append(includeOnlyKeys, dbKey.KeyID)
+				}
+				ctx.SetValue(schemas.BifrostContextKeyGovernanceIncludeOnlyKeys, includeOnlyKeys)
 			}
-			ctx.SetValue(schemas.BifrostContextKeyGovernanceIncludeOnlyKeys, includeOnlyKeys)
 			break
 		}
 	}
 
-	// Skip rate limits and budgets — user auth handles those
+	// All checks passed
 	return &EvaluationResult{
 		Decision:   DecisionAllow,
-		Reason:     "Request allowed by governance policy (VK filtering only)",
+		Reason:     "Request allowed by governance policy",
 		VirtualKey: vk,
 	}
 }