From c7c9565d49e328788de741f3cc7a5561c9941932 Mon Sep 17 00:00:00 2001 From: Pratham-Mishra04 Date: Mon, 20 Jul 2026 12:22:20 +0530 Subject: [PATCH] fix: validate routing rule scope_id references a real entity --- .../bifrost-http/handlers/governance.go | 59 +++++++++++++++ transports/bifrost-http/lib/config.go | 73 +++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/transports/bifrost-http/handlers/governance.go b/transports/bifrost-http/handlers/governance.go index c23c51278fa..bdb82a2800a 100644 --- a/transports/bifrost-http/handlers/governance.go +++ b/transports/bifrost-http/handlers/governance.go @@ -4028,6 +4028,9 @@ func (h *GovernanceHandler) createRoutingRule(ctx *fasthttp.RequestCtx) { } else if req.ScopeID == nil || *req.ScopeID == "" { SendError(ctx, 400, "scope_id field is required when scope is not global") return + } else if err := h.validateRoutingScopeID(ctx, scope, *req.ScopeID); err != nil { + sendRoutingScopeIDValidationError(ctx, err) + return } // Build targets @@ -4180,6 +4183,13 @@ func (h *GovernanceHandler) updateRoutingRule(ctx *fasthttp.RequestCtx) { } else if rule.ScopeID == nil || *rule.ScopeID == "" { SendError(ctx, 400, "scope_id field is required when scope is not global") return + } else if req.Scope != nil || req.ScopeID != nil { + // Only re-validate when scope or scope_id actually changed in this request; + // avoids re-checking on every unrelated update (e.g. toggling enabled). + if err := h.validateRoutingScopeID(ctx, rule.Scope, *rule.ScopeID); err != nil { + sendRoutingScopeIDValidationError(ctx, err) + return + } } // Update in database @@ -4602,6 +4612,55 @@ var validRoutingScopes = map[string]bool{ "virtual_key": true, } +// errRoutingScopeIDNotFound marks a validateRoutingScopeID failure as a genuine +// "entity doesn't exist" rejection, as opposed to a store error (DB down, timeout, +// context cancellation). Callers use errors.Is to tell the two apart: the former +// is a 400 (bad request data), the latter a 500 (server couldn't verify). +var errRoutingScopeIDNotFound = errors.New("routing rule scope_id not found") + +// validateRoutingScopeID checks that scopeID resolves to an existing entity of the +// given scope type. A rule whose scope_id doesn't resolve silently matches zero +// requests (the routing engine caches rules keyed by the real entity ID), so this +// must be rejected at write time rather than left to fail invisibly at eval time. +func (h *GovernanceHandler) validateRoutingScopeID(ctx context.Context, scope string, scopeID string) error { + switch scope { + case "virtual_key": + if _, err := h.configStore.GetVirtualKey(ctx, scopeID); err != nil { + if errors.Is(err, configstore.ErrNotFound) { + return fmt.Errorf("virtual key '%s' not found: %w", scopeID, errRoutingScopeIDNotFound) + } + return fmt.Errorf("failed to verify virtual key: %w", err) + } + case "team": + if _, err := h.configStore.GetTeam(ctx, scopeID); err != nil { + if errors.Is(err, configstore.ErrNotFound) { + return fmt.Errorf("team '%s' not found: %w", scopeID, errRoutingScopeIDNotFound) + } + return fmt.Errorf("failed to verify team: %w", err) + } + case "customer": + if _, err := h.configStore.GetCustomer(ctx, scopeID); err != nil { + if errors.Is(err, configstore.ErrNotFound) { + return fmt.Errorf("customer '%s' not found: %w", scopeID, errRoutingScopeIDNotFound) + } + return fmt.Errorf("failed to verify customer: %w", err) + } + } + return nil +} + +// sendRoutingScopeIDValidationError maps a validateRoutingScopeID error to the +// right HTTP status: 400 when scope_id genuinely doesn't resolve, 500 when the +// store itself failed and existence couldn't be determined. +func sendRoutingScopeIDValidationError(ctx *fasthttp.RequestCtx, err error) { + if errors.Is(err, errRoutingScopeIDNotFound) { + SendError(ctx, 400, err.Error()) + return + } + logger.Error("failed to validate routing rule scope_id: %v", err) + SendError(ctx, 500, "Failed to verify scope_id") +} + // validateRoutingScope validates that the scope value is one of the allowed values func validateRoutingScope(scope string) error { if scope == "" { diff --git a/transports/bifrost-http/lib/config.go b/transports/bifrost-http/lib/config.go index 2af7779d1c6..7d2da30cece 100644 --- a/transports/bifrost-http/lib/config.go +++ b/transports/bifrost-http/lib/config.go @@ -2335,6 +2335,20 @@ func resolveGovernanceKeyReferences(ctx context.Context, config *Config, governa } // mergeGovernanceConfig merges governance config from file with store +// entityIDSet builds a lookup set of IDs from an already-synced slice plus a +// slice of entries still pending insertion, so newly-added-in-this-file entities +// are valid scope_id targets even before they're persisted. +func entityIDSet[T any](existing []T, id func(T) string, toAdd []T) map[string]bool { + set := make(map[string]bool, len(existing)+len(toAdd)) + for _, e := range existing { + set[id(e)] = true + } + for _, e := range toAdd { + set[id(e)] = true + } + return set +} + func mergeGovernanceConfig(ctx context.Context, config *Config, configData *ConfigData, governanceConfig *configstore.GovernanceConfig) { logger.Debug("merging governance config from config file with store") // When config.json is the source of truth, file-present entities must be @@ -2552,6 +2566,65 @@ func mergeGovernanceConfig(ctx context.Context, config *Config, configData *Conf virtualKeysToAdd = append(virtualKeysToAdd, configData.Governance.VirtualKeys[i]) } } + // Build the set of entity IDs each non-global scope may reference, so a routing + // rule whose scope_id doesn't resolve (e.g. a name typed in place of an ID) is + // rejected here instead of silently matching zero requests at eval time. + // + // Under source_of_truth=config.json, entities absent from the file are deleted + // by the later prune step, so a currently-persisted entity that isn't in the + // file is NOT a valid target — only file-declared IDs survive. Otherwise + // (default merge mode, or the section is simply absent from this file) anything + // already persisted, or being added by this same file, remains valid. + teamScopeIDs := entityIDSet(governanceConfig.Teams, func(t configstoreTables.TableTeam) string { return t.ID }, teamsToAdd) + if configData.isConfigJSONSourceOfTruth() && configData.governanceSectionPresent("teams") { + teamScopeIDs = entityIDSet(configData.Governance.Teams, func(t configstoreTables.TableTeam) string { return t.ID }, nil) + } + customerScopeIDs := entityIDSet(governanceConfig.Customers, func(c configstoreTables.TableCustomer) string { return c.ID }, customersToAdd) + if configData.isConfigJSONSourceOfTruth() && configData.governanceSectionPresent("customers") { + customerScopeIDs = entityIDSet(configData.Governance.Customers, func(c configstoreTables.TableCustomer) string { return c.ID }, nil) + } + vkScopeIDs := entityIDSet(governanceConfig.VirtualKeys, func(vk configstoreTables.TableVirtualKey) string { return vk.ID }, virtualKeysToAdd) + if configData.isConfigJSONSourceOfTruth() && configData.governanceSectionPresent("virtual_keys") { + vkScopeIDs = entityIDSet(configData.Governance.VirtualKeys, func(vk configstoreTables.TableVirtualKey) string { return vk.ID }, nil) + } + validRoutingScopeIDs := map[string]map[string]bool{ + "team": teamScopeIDs, + "customer": customerScopeIDs, + "virtual_key": vkScopeIDs, + } + + // Sanitize routing rules with an unresolvable scope_id BEFORE the merge loop + // and before pruneGovernanceConfigToFile treats configData.Governance.RoutingRules + // as the new authoritative snapshot (it both computes the prune keep-set from it + // and assigns it verbatim to config.GovernanceConfig.RoutingRules). A brand-new + // invalid rule is dropped outright; an invalid edit to an existing rule reverts + // to the last persisted version, so the in-memory snapshot and the prune + // keep-set never reflect data that was rejected and never written to the DB. + existingRoutingRulesByID := make(map[string]configstoreTables.TableRoutingRule, len(governanceConfig.RoutingRules)) + for _, r := range governanceConfig.RoutingRules { + existingRoutingRulesByID[r.ID] = r + } + sanitizedRoutingRules := make([]configstoreTables.TableRoutingRule, 0, len(configData.Governance.RoutingRules)) + for _, rule := range configData.Governance.RoutingRules { + if rule.Scope != "" && rule.Scope != "global" { + scopeID := "" + if rule.ScopeID != nil { + scopeID = *rule.ScopeID + } + if scopeID == "" || !validRoutingScopeIDs[rule.Scope][scopeID] { + if existing, ok := existingRoutingRulesByID[rule.ID]; ok { + logger.Warn("routing rule %s: scope_id %q does not resolve to an existing %s (use the entity's id, not its name); keeping last persisted version", rule.ID, scopeID, rule.Scope) + sanitizedRoutingRules = append(sanitizedRoutingRules, existing) + } else { + logger.Warn("routing rule %s: scope_id %q does not resolve to an existing %s (use the entity's id, not its name); skipping new rule", rule.ID, scopeID, rule.Scope) + } + continue + } + } + sanitizedRoutingRules = append(sanitizedRoutingRules, rule) + } + configData.Governance.RoutingRules = sanitizedRoutingRules + // Merge RoutingRules by ID with hash comparison routingRulesToAdd := make([]configstoreTables.TableRoutingRule, 0) routingRulesToUpdate := make([]configstoreTables.TableRoutingRule, 0)