restrict fallbacks and provider selection to vk boundry

[akshaydeo]

Summary

When a virtual key has provider constraints (but no weights), the governance plugin now propagates the list of allowed providers into the Bifrost context. The router then intersects that list with the model catalog's provider list, ensuring that only providers permitted by the virtual key are considered for routing and fallbacks.

Changes

• The governance plugin sets BifrostContextKeyAvailableProviders on the context after filtering provider configs, including setting an empty slice when no provider configs exist or when no providers pass the model filter.
• The router's createHandler intersects the catalog-derived provider list with any pre-existing BifrostContextKeyAvailableProviders value set by upstream plugins (e.g., governance). If the intersection is empty, an empty provider list is stored rather than falling back to the full catalog set.
• extractAndParseFallbacks now accepts a BifrostContext and filters parsed fallbacks to only those whose provider appears in BifrostContextKeyAvailableProviders. If all fallbacks are filtered out, the fallback list on the request is explicitly cleared to nil.

Type of change

• Bug fix
• Feature
• Refactor
• Documentation
• Chore/CI

Affected areas

• Core (Go)
• Transports (HTTP)
• Providers/Integrations
• Plugins

How to test

go test ./plugins/governance/...
go test ./transports/bifrost-http/integrations/...

• A virtual key with openai/gpt-4o and anthropic/claude-3-5-sonnet provider configs (no weights) and a request for gpt-4o should result in BifrostContextKeyAvailableProviders containing only openai.
• A virtual key with only openai/gpt-4o and a request for claude-3-5-sonnet should result in an empty BifrostContextKeyAvailableProviders.
• A request with fallbacks that include providers not in the allowed list should have those fallbacks stripped before the request is dispatched.

Breaking changes

• Yes
• No

Related issues

Closes #2516

Security considerations

Provider constraints enforced by virtual keys are now respected end-to-end through routing and fallback resolution, preventing requests from being routed to providers that the virtual key does not permit.

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

Summary by CodeRabbit

Release Notes

• Tests

  • Added comprehensive test coverage for governance HTTP transport pre-hook with provider-constrained virtual keys.
  • Added router tests verifying proper provider constraint enforcement during request handling.

• Bug Fixes

  • Router now correctly respects provider availability constraints when selecting providers for requests.
  • Fallback extraction now filters to only providers permitted by governance constraints.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 0033cb91-5777-4e82-91f5-e81d5eba2aef

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR threads provider availability constraints through the request handling pipeline. The governance plugin now sets allowed providers from virtual key constraints into the Bifrost context, the router respects this constraint when selecting catalog providers, and fallback extraction filters fallbacks accordingly.

Changes

Provider Availability Constraint Through Request Pipeline

Layer / File(s)	Summary
Governance provider constraint initialization plugins/governance/main.go, plugins/governance/httptransportprehook_test.go	loadBalanceProvider now sets BifrostContextKeyAvailableProviders from virtual key provider configs; tests verify the context is populated when model matches allowed providers and left empty when no provider allows the model.
Router provider selection respects governance constraints transports/bifrost-http/integrations/router.go, transports/bifrost-http/integrations/router_test.go	Router model-catalog logic now reads pre-existing BifrostContextKeyAvailableProviders and intersects catalog results with that constraint; propagates the constrained list back to context; call to extractAndParseFallbacks now passes the bifrost context.
Fallback extraction filters by available providers transports/bifrost-http/integrations/utils.go, transports/bifrost-http/integrations/utils_test.go	extractAndParseFallbacks now accepts BifrostContext parameter and filters parsed fallback providers to only those in AvailableProviders; tests verify filtering behavior and that invalid fallbacks are cleared.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

• danpiths

Poem

🐰 A governance key sets providers free,
Router selects what's meant to be,
Fallbacks constrain with context tight,
Three layers thread the flow just right! ✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Linked Issues check	⚠️ Warning	Linked issue #123 requires Files API support for providers (OpenAI, Anthropic) for uploading files. The PR changes focus on restricting provider selection and fallbacks to virtual key boundaries, which is unrelated to File API implementation.	The PR does not address Files API requirements from issue #123. Either the PR should implement file upload endpoints, or the issue link should be corrected to match the actual objective (restricting provider selection to virtual key boundaries).
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title references 'vk boundry' (virtual key boundary) and mentions restricting fallbacks and provider selection, which directly aligns with the core changes of enforcing virtual key provider constraints throughout the system.
Description check	✅ Passed	The PR description covers Summary, Changes, Type of change, Affected areas, How to test, Breaking changes, and Related issues sections from the template. Most key details are present, though documentation updates are marked incomplete.
Out of Scope Changes check	✅ Passed	All code changes (governance plugin updates, router provider filtering, fallback filtering, and tests) are cohesively scoped to enforcing virtual key provider constraints end-to-end, with no extraneous modifications detected.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 05-31-restrict_fallbacks_and_provider_selection_to_vk_boundry

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[akshaydeo]

• coderabbit yml changes #3975
• handles compaction message type #3966
• openai integration content string handling #3949
• logs adds support for rendering file attachments #3931
• capture resolved provider from the loadbalancer for logging #3930
• restrict fallbacks and provider selection to vk boundry #3924 👈 (View in Graphite)
• azure dignotstic property strip for claude models #3925
• dev

This stack of pull requests is managed by Graphite. Learn more about stacking.

[greptile-apps]

Confidence Score: 3/5

The core constraint propagation mechanism works correctly in the happy path, but two previously-flagged structural gaps mean the VK provider boundary is not reliably enforced for fallbacks: the router narrows the allowed-provider list to the route's native provider before the fallback filter reads it, and governance's early-return for provider-prefixed models skips populating the context key entirely.

Both flagged gaps produce silent mis-enforcement: a VK explicitly permitting multiple providers will still drop valid cross-provider fallbacks after the router's single-provider narrowing step, and any request using a slash-prefixed primary model bypasses constraint propagation to the fallback stage completely. These are real, reproducible behavioral defects on the changed code paths rather than theoretical risks.

transports/bifrost-http/integrations/router.go (lines 815–828, where the context key is overwritten to a single-element list before fallback extraction) and plugins/governance/main.go (lines 740–751, the early return for provider-prefixed models that skips setting the context key).

Important Files Changed

Filename	Overview
plugins/governance/main.go	Sets BifrostContextKeyAvailableProviders in two new places; the early-return path for provider-prefixed models still exits without setting the key, leaving fallback filtering inactive for that code path (previously flagged).
transports/bifrost-http/integrations/router.go	Intersection logic is correct, but the key is overwritten to a single-element list (the route's native provider) before extractAndParseFallbacks runs, making the fallback filter too narrow (previously flagged).
transports/bifrost-http/integrations/utils.go	extractAndParseFallbacks correctly filters by available providers and now calls SetFallbacks(nil) to clear pre-existing fallbacks when all candidates are denied; GetProvidersForModel returns a fresh slice so slices.DeleteFunc is safe.
plugins/governance/httptransportprehook_test.go	Two new test cases cover the model-filtered single-provider and no-match-provider scenarios; test setup is correct — weight=nil configs exercise the constraint-only path.
transports/bifrost-http/integrations/router_test.go	New tests correctly simulate governance-set context values via fasthttp SetUserValue (copied to bifrostCtx by ConvertToBifrostContext) and verify the catalog–VK intersection; assertions are accurate.
transports/bifrost-http/integrations/utils_test.go	New tests cover provider filtering and SetFallbacks(nil) clearing pre-parsed fallbacks; SetFallbacks correctly handles the nil case via the non-nil sub-request switch in core/schemas/bifrost.go.

_{Reviews (7): Last reviewed commit: "restrict fallbacks and provider selectio..." | Re-trigger Greptile}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

plugins/governance/main.go (1)

739-761: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Preserve VK provider constraints on prefixed-model requests before early return.

Line 740-Line 751 can return before Line 758/Line 827 are reached. That skips BifrostContextKeyAvailableProviders propagation for provider-prefixed models, so downstream routing/fallback filtering may miss VK boundaries on this path.

🔧 Suggested direction

-	// Check if model already has provider prefix (contains "/")
-	if strings.Contains(modelStr, "/") {
-		provider, _ := schemas.ParseModelString(modelStr, "")
-		// Checking valid provider when store is available; if store is nil,
-		// assume the prefixed model should be left unchanged.
-		if p.inMemoryStore != nil {
-			if _, ok := p.inMemoryStore.GetConfiguredProviders()[provider]; ok {
-				return body, nil
-			}
-		} else {
-			return body, nil
-		}
-	}
+	// Keep track of provider-pinned input, but still compute/set available providers first
+	alreadyPinnedProvider := false
+	if strings.Contains(modelStr, "/") {
+		provider, _ := schemas.ParseModelString(modelStr, "")
+		if p.inMemoryStore == nil {
+			alreadyPinnedProvider = true
+		} else if _, ok := p.inMemoryStore.GetConfiguredProviders()[provider]; ok {
+			alreadyPinnedProvider = true
+		}
+	}
@@
 	ctx.SetValue(schemas.BifrostContextKeyAvailableProviders, allowedModelProviders)
@@
+	// Model already pinned by caller: keep model unchanged, but constraints are now propagated.
+	if alreadyPinnedProvider {
+		return body, nil
+	}

As per coding guidelines, virtual_keys[].provider_configs: empty array = no providers allowed (deny-by-default) and the effective result should behave like no allowed providers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@plugins/governance/main.go` around lines 739 - 761, The early return for
prefixed models in the code that checks modelStr (the block using
strings.Contains(modelStr), schemas.ParseModelString, and p.inMemoryStore) skips
setting the per-virtual-key available providers context (ctx.SetValue with
schemas.BifrostContextKeyAvailableProviders) and its warning log, which breaks
VK provider constraints for prefixed requests; fix by, before any return in that
prefixed-model branch, computing and setting the available providers from
virtualKey.ProviderConfigs (if len(virtualKey.ProviderConfigs)==0 set an empty
[]schemas.ModelProvider and append the warn log via ctx.AppendRoutingEngineLog),
then return body,nil as before so downstream routing/fallback sees the VK
constraints.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@plugins/governance/main.go`:
- Around line 739-761: The early return for prefixed models in the code that
checks modelStr (the block using strings.Contains(modelStr),
schemas.ParseModelString, and p.inMemoryStore) skips setting the per-virtual-key
available providers context (ctx.SetValue with
schemas.BifrostContextKeyAvailableProviders) and its warning log, which breaks
VK provider constraints for prefixed requests; fix by, before any return in that
prefixed-model branch, computing and setting the available providers from
virtualKey.ProviderConfigs (if len(virtualKey.ProviderConfigs)==0 set an empty
[]schemas.ModelProvider and append the warn log via ctx.AppendRoutingEngineLog),
then return body,nil as before so downstream routing/fallback sees the VK
constraints.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: aa0fa3b8-75d8-478b-b9bd-c3748f0c8dce

📥 Commits

Reviewing files that changed from the base of the PR and between d4c96b8 and ee05752.

📒 Files selected for processing (6)

• plugins/governance/httptransportprehook_test.go
• plugins/governance/main.go
• transports/bifrost-http/integrations/router.go
• transports/bifrost-http/integrations/router_test.go
• transports/bifrost-http/integrations/utils.go
• transports/bifrost-http/integrations/utils_test.go

[akshaydeo]

Merge activity

• Jun 2, 8:41 AM UTC: A user started a stack merge that includes this pull request via Graphite.
• Jun 2, 8:42 AM UTC: @akshaydeo merged this pull request with Graphite.