Skip to content

fix(governance): enforce vk blocked models - #3718

Merged
akshaydeo merged 1 commit into
devfrom
fix/bf-896-vk-blocked-models-editor-clean
May 25, 2026
Merged

fix(governance): enforce vk blocked models#3718
akshaydeo merged 1 commit into
devfrom
fix/bf-896-vk-blocked-models-editor-clean

Conversation

@Vaibhav701161

Copy link
Copy Markdown
Contributor

Summary

Adds the missing create/edit UI for VK-level blocked models and fixes runtime enforcement for provider-prefixed model names.

The previous VK blocked-model support had backend storage, API handling, runtime hooks, and read-only display, but the create/edit sheet did not expose the Blocked Models editor. This PR adds that missing UI field and also fixes an enforcement issue found during E2E testing: the UI stores selected models as provider/model, while the governance resolver could compare against bare model names. Because of that mismatch, blocked models could bypass the VK blacklist in some request paths.

Changes

  • Added editable Blocked Models field in the VK create/edit provider config section.

    • Placed between Allowed Models and Allowed Keys.
    • Uses the same ModelMultiselect pattern as the allowed models field.
    • Adds blacklisted_models to the zod schema.
    • Initializes edit mode with config.blacklisted_models || [].
    • Defaults new provider configs to blacklisted_models: [].
  • Added prefix-aware blocked-model matching in governance.

    • Added isModelBlockedByList() to handle both bare and provider-prefixed model names.
    • This prevents bypasses where the UI stores ollama/mistral:latest, but the runtime request is evaluated as mistral:latest, or the other way around.
  • Updated VK runtime enforcement paths to use the prefix-aware matcher:

    • plugins/governance/resolver.go
    • plugins/governance/main.go
    • plugins/governance/utils.go

Design decision:

  • The fix keeps the existing schemas.BlackList behavior intact and adds a small VK-specific helper for prefix-aware comparison.
  • Provider-key behavior is unchanged.

Type of change

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

Affected areas

  • Core (Go)
  • Transports (HTTP)
  • Providers/Integrations
  • Plugins
  • UI (React)
  • Docs

How to test

Build checks:

go build -o ./tmp/bifrost-http ./transports/bifrost-http

cd ui
npm run build

Local server:

./tmp/bifrost-http -port 9090

Open:

http://localhost:9090/workspace/governance/virtual-keys

UI validation:

  1. Open Governance → Virtual Keys.

  2. Create or edit a virtual key.

  3. Expand a provider config.

  4. Confirm the field order is:

    • Allowed Models
    • Blocked Models
    • Allowed Keys
  5. Select a blocked model and save.

  6. Reopen the virtual key and confirm the blocked model is still selected.

  7. Open the details sheet and confirm the blocked model is displayed.

E2E validation with local Ollama:

  • Blocked bare model: 403 model_blocked
  • Blocked provider-prefixed model: 403 model_blocked
  • Allowed bare model: 200 OK
  • Allowed provider-prefixed model: 200 OK
  • Same model in allowlist and blocklist: 403 model_blocked
  • Empty blocklist: 200 OK
  • Wildcard blocklist ["*"]: all tested models return 403 model_blocked
  • UI-created VK also works end-to-end with the same blocked/allowed behavior

Screenshots/Recordings

Added/available recording showing the Blocked Models field in the VK provider config create/edit flow.

Uploading Screen Recording 2026-05-25 003101.mp4…

Breaking changes

  • Yes
  • No

Related issues

BF-896

Security considerations

This improves VK-level governance by ensuring blocked models are actually enforced for both bare and provider-prefixed model strings.

No secrets, provider keys, auth tokens, or PII are exposed or stored by this change. Provider-key behavior is unchanged.

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

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Provider-level blocked models (denylist) configuration in virtual key settings, with wildcard (*) support.
  • Refactor

    • Improved blacklist matching for consistent, case-insensitive model comparisons across governance checks.
  • Tests

    • Added unit tests covering blacklist matching scenarios (wildcard, exact, and provider-prefixed vs bare model forms).

Walkthrough

This PR adds a normalized blacklist helper isModelBlockedByList, replaces three .IsBlocked() calls in governance filtering to use it, includes unit tests for matching behavior, and extends the virtual-key provider config and UI with a blacklisted_models field and "Blocked Models" multiselect (wildcard-capable).

Changes

Provider blacklisted models feature

Layer / File(s) Summary
Backend blacklist matching helper and integration
plugins/governance/utils.go, plugins/governance/main.go, plugins/governance/resolver.go, plugins/governance/utils.go test
Adds isModelBlockedByList (case-insensitive, supports wildcard/block-all and bare vs provider-prefixed equivalence via schemas.ParseModelString) and updates loadBalanceProvider, isModelAllowed, and filterModelsForVirtualKey to call it. Adds plugins/governance/blocklist_test.go covering wildcard, exact, and prefixed/bare matching.
Frontend denylist configuration schema and UI
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx
Adds optional blacklisted_models to providerConfig schema, initializes defaults and new-provider objects with blacklisted_models: [], and inserts a "Blocked Models" accordion section rendering ModelMultiselect with * handling that updates blacklisted_models.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • maximhq/bifrost#3653: Related work on virtual-key blacklisted_models denylist precedence and virtual-key blacklist handling.

Suggested reviewers

  • danpiths
  • roroghost17

Poem

🐰 I hopped through code to make lists clear,
I normalized names both far and near,
Wildcards and prefixes I gently align,
A helper now guards each denied design,
Cheers to denylists — tidy and dear!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(governance): enforce vk blocked models' clearly and concisely describes the main change—fixing Virtual Key blocked model enforcement—and accurately reflects the primary objective of the PR.
Description check ✅ Passed The PR description comprehensively covers all major template sections: summary, detailed changes, type of change, affected areas, testing steps, screenshots note, breaking changes, related issues, and security considerations. The description is complete and well-organized.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/bf-896-vk-blocked-models-editor-clean

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
plugins/governance/resolver.go (1)

15-32: ⚡ Quick win

Add focused table-driven tests for the new blacklist matcher.

This helper now defines denylist semantics reused across governance paths; please add tests for bare match, provider-prefixed match, wildcard "*", and case-insensitive comparisons to lock behavior.

🤖 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/resolver.go` around lines 15 - 32, Add focused
table-driven unit tests for isModelBlockedByList that exercise denylist
semantics: create cases for (1) exact bare model match (e.g., blacklist contains
"gpt-4", model "gpt-4"), (2) provider-prefixed entries (e.g., blacklist contains
"gemini/gemini-2.0-flash-lite", model "gemini-2.0-flash-lite"), (3) wildcard
entry "*" which should block any model, and (4) case-insensitive matches (e.g.,
blacklist "GEMINI/GEMINI-2.0", model "gemini-2.0"); also include a non-matching
case and a case where blacklist.IsBlockAll() returns true to ensure immediate
true. For each table row assert isModelBlockedByList(blacklist, model) returns
the expected boolean, and construct BlackList instances and, where needed, use
schemas.ParseModelString to mirror production parsing behavior; reference the
isModelBlockedByList function, BlackList type and its IsBlockAll method, and
schemas.ParseModelString in your tests so they cover all described behaviors.
🤖 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.

Nitpick comments:
In `@plugins/governance/resolver.go`:
- Around line 15-32: Add focused table-driven unit tests for
isModelBlockedByList that exercise denylist semantics: create cases for (1)
exact bare model match (e.g., blacklist contains "gpt-4", model "gpt-4"), (2)
provider-prefixed entries (e.g., blacklist contains
"gemini/gemini-2.0-flash-lite", model "gemini-2.0-flash-lite"), (3) wildcard
entry "*" which should block any model, and (4) case-insensitive matches (e.g.,
blacklist "GEMINI/GEMINI-2.0", model "gemini-2.0"); also include a non-matching
case and a case where blacklist.IsBlockAll() returns true to ensure immediate
true. For each table row assert isModelBlockedByList(blacklist, model) returns
the expected boolean, and construct BlackList instances and, where needed, use
schemas.ParseModelString to mirror production parsing behavior; reference the
isModelBlockedByList function, BlackList type and its IsBlockAll method, and
schemas.ParseModelString in your tests so they cover all described behaviors.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 86c4d6d2-618e-45a4-bae6-332d57d84e73

📥 Commits

Reviewing files that changed from the base of the PR and between ff463d9 and dfb9629.

📒 Files selected for processing (4)
  • plugins/governance/main.go
  • plugins/governance/resolver.go
  • plugins/governance/utils.go
  • ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 24, 2026
@greptile-apps

greptile-apps Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge — the core enforcement fix is sound and the UI change is additive.

The prefix-aware matching logic in isModelBlockedByList correctly handles the bare↔prefixed cases that the PR targets. The three call-sites in resolver, main, and utils all use the new helper consistently. The cross-provider normalization edge case in the test file is a documentation gap rather than a behavioral regression.

No files require special attention.

Important Files Changed

Filename Overview
plugins/governance/utils.go Adds isModelBlockedByList — normalizes both the blacklist entry and the incoming model via ParseModelString before comparing, correctly handling prefixed↔bare and bare↔prefixed matches. Cross-provider prefixed entries (e.g., ollama/model matching openai/model) can produce false positives due to prefix-stripping, but the scenario requires a misconfiguration.
plugins/governance/resolver.go Replaces BlacklistedModels.IsBlocked(model) with isModelBlockedByList(...) in the VK allowlist check; the pc.Provider == string(provider) guard already scopes the call to the right config, limiting exposure to cross-provider false positives.
plugins/governance/main.go One-line change in loadBalanceProvider to use the prefix-aware helper; provider isolation is maintained because each config is keyed to its own provider and the blacklisted-providers map is indexed by provider.
plugins/governance/blocklist_test.go Adds table-driven tests for isModelBlockedByList; covers the main prefix-matching cases but is missing a cross-provider test (e.g., ollama/mistral in blocklist vs openai/mistral incoming) that would document the normalization trade-off.
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx Adds blacklisted_models field to the Zod schema and UI; wildcard handling mirrors the allowed-models pattern and looks correct; field is properly initialized for both create and edit modes.

Reviews (2): Last reviewed commit: "fix(governance): enforce vk blocked mode..." | Re-trigger Greptile

Comment thread plugins/governance/resolver.go Outdated
Comment thread plugins/governance/resolver.go Outdated
Comment thread plugins/governance/resolver.go Outdated
Add missing create/edit UI for VK-level blocked models and fix runtime
enforcement to handle provider-prefixed model names.

The UI (ModelMultiselect) stores models as "provider/model" but the
governance resolver compared against bare model names, so no model was
ever actually blocked. Add isModelBlockedByList() which strips the
provider prefix before comparing — mirrors the prefix-aware logic
already present in IsModelAllowedForProvider for the allowlist.

Changes:
- ui/virtualKeySheet.tsx: add editable Blocked Models field in VK
  create/edit provider config section (between Allowed Models and
  Allowed Keys). Adds zod schema field, edit-mode init with
  blacklisted_models || [], and default [] for new configs.
- plugins/governance/resolver.go: add isModelBlockedByList() helper;
  use it in isModelAllowed() blacklist pass.
- plugins/governance/main.go: use isModelBlockedByList() in
  loadBalanceProvider pre-pass.
- plugins/governance/utils.go: use isModelBlockedByList() in
  filterModelsForVirtualKey.

Verified E2E with local Ollama: blocked model → 403, allowed model →
200, blacklist-over-allowlist → 403, empty blacklist → 200, wildcard
blacklist → 403, bare and prefixed model strings both blocked correctly.

Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>
@Vaibhav701161
Vaibhav701161 force-pushed the fix/bf-896-vk-blocked-models-editor-clean branch from dfb9629 to 582c721 Compare May 25, 2026 05:24

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 (1)
plugins/governance/main.go (1)

698-707: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Replace raw string context keys with typed context key identifiers

  • plugins/governance/main.go uses raw string BifrostContext keys for both reads and writes: ctx.Value("model") / ctx.Value("modelId") and ctx.SetValue("model", ...) / ctx.SetValue("modelId", ...) (e.g., lines 698-707, 875-878, 1032-1040).
  • Update callers and tests to use dedicated typed key identifiers (e.g., plugins/governance/httptransportprehook_test.go, and core/schemas/context_test.go).
🤖 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 698 - 707, The code is using raw
string context keys; replace all uses of ctx.Value("model"),
ctx.Value("modelId"), ctx.SetValue("model", ...), and ctx.SetValue("modelId",
...) in plugins/governance/main.go with the project’s typed context key
identifiers (e.g., the exported key types defined in core/schemas/context or the
package that declares them); update any callers and tests
(plugins/governance/httptransportprehook_test.go and
core/schemas/context_test.go) to read/write using those typed keys instead of
raw strings so context access is type-safe and consistent across the codebase.
🧹 Nitpick comments (2)
plugins/governance/blocklist_test.go (1)

16-57: ⚡ Quick win

Add explicit case-insensitive test coverage for blacklist matching.

The helper compares using case-insensitive equality, but the table currently doesn’t lock that behavior with a mixed-case input case.

✅ Suggested test additions
 		{
 			name:      "prefixed blocks prefixed",
 			blacklist: schemas.BlackList{"ollama/mistral:latest"},
 			model:     "ollama/mistral:latest",
 			want:      true,
 		},
+		{
+			name:      "case-insensitive bare match",
+			blacklist: schemas.BlackList{"MISTRAL:LATEST"},
+			model:     "mistral:latest",
+			want:      true,
+		},
+		{
+			name:      "case-insensitive prefixed vs bare match",
+			blacklist: schemas.BlackList{"OLLAMA/MISTRAL:LATEST"},
+			model:     "mistral:latest",
+			want:      true,
+		},
 		{
 			name:      "different model not blocked",
 			blacklist: schemas.BlackList{"mistral:latest"},
 			model:     "llama3.2:latest",
 			want:      false,
 		},
🤖 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/blocklist_test.go` around lines 16 - 57, Add explicit
case-insensitive test rows to the existing table in blocklist_test.go so
matching is validated regardless of case: add entries where blacklist contains
mixed-case values (e.g., schemas.BlackList{"MiStRaL:Latest"} or
{"OLLAMA/MiStRaL:Latest"}) and model contains the lower-case form
("mistral:latest") and vice versa (blacklist lower-case, model mixed-case), and
ensure want is true; also add a mixed-case wildcard ("*") check to confirm
wildcard remains effective. These new rows should target the same test harness
that iterates over the table (using schemas.BlackList and model fields) so the
helper’s case-insensitive comparison behavior is covered.
ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx (1)

1452-1472: ⚡ Quick win

Extract provider-key resolution into a shared helper to avoid drift.

The keys={(() => { ... })()} block here duplicates the same logic used in the allowed-models section. A shared helper will keep allow/block selectors behavior consistent when this logic changes.

🤖 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 `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx` around lines 1452 -
1472, Extract the provider-key resolution logic out of the inline IIFE into a
shared helper (e.g., resolveProviderKeyIds(availableKeys, config)) and replace
the inline keys={(() => { ... })()} with
keys={resolveProviderKeyIds(availableKeys, config)}; the helper should accept
availableKeys and the config (reading config.provider and config.key_ids or
defaulting to []) and return either all provider key_ids when key_ids includes
"*" or the intersection of config.key_ids and providerKeys' key_id values.
Update the corresponding allowed-models resolution to call the same helper so
both selectors use identical logic.
🤖 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.

Inline comments:
In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1428-1431: The new TooltipTrigger interactive element
(TooltipTrigger asChild) in virtualKeySheet.tsx lacks a test selector; update
the trigger to include a data-testid attribute (e.g.,
data-testid="blocked-models-tooltip-trigger") on the asChild wrapper element
(the <span> wrapping the Info icon) so E2E tests can reliably target it; ensure
the identifier is unique and follows existing testid naming conventions used in
this component.

---

Outside diff comments:
In `@plugins/governance/main.go`:
- Around line 698-707: The code is using raw string context keys; replace all
uses of ctx.Value("model"), ctx.Value("modelId"), ctx.SetValue("model", ...),
and ctx.SetValue("modelId", ...) in plugins/governance/main.go with the
project’s typed context key identifiers (e.g., the exported key types defined in
core/schemas/context or the package that declares them); update any callers and
tests (plugins/governance/httptransportprehook_test.go and
core/schemas/context_test.go) to read/write using those typed keys instead of
raw strings so context access is type-safe and consistent across the codebase.

---

Nitpick comments:
In `@plugins/governance/blocklist_test.go`:
- Around line 16-57: Add explicit case-insensitive test rows to the existing
table in blocklist_test.go so matching is validated regardless of case: add
entries where blacklist contains mixed-case values (e.g.,
schemas.BlackList{"MiStRaL:Latest"} or {"OLLAMA/MiStRaL:Latest"}) and model
contains the lower-case form ("mistral:latest") and vice versa (blacklist
lower-case, model mixed-case), and ensure want is true; also add a mixed-case
wildcard ("*") check to confirm wildcard remains effective. These new rows
should target the same test harness that iterates over the table (using
schemas.BlackList and model fields) so the helper’s case-insensitive comparison
behavior is covered.

In `@ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx`:
- Around line 1452-1472: Extract the provider-key resolution logic out of the
inline IIFE into a shared helper (e.g., resolveProviderKeyIds(availableKeys,
config)) and replace the inline keys={(() => { ... })()} with
keys={resolveProviderKeyIds(availableKeys, config)}; the helper should accept
availableKeys and the config (reading config.provider and config.key_ids or
defaulting to []) and return either all provider key_ids when key_ids includes
"*" or the intersection of config.key_ids and providerKeys' key_id values.
Update the corresponding allowed-models resolution to call the same helper so
both selectors use identical logic.
🪄 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: 75d59fa8-ca83-4cf7-a8ad-c567a2abe8d9

📥 Commits

Reviewing files that changed from the base of the PR and between dfb9629 and 582c721.

📒 Files selected for processing (5)
  • plugins/governance/blocklist_test.go
  • plugins/governance/main.go
  • plugins/governance/resolver.go
  • plugins/governance/utils.go
  • ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx

Comment thread ui/app/workspace/virtual-keys/views/virtualKeySheet.tsx

akshaydeo commented May 25, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • May 25, 7:19 AM UTC: A user started a stack merge that includes this pull request via Graphite.
  • May 25, 7:19 AM UTC: @akshaydeo merged this pull request with Graphite.

@akshaydeo
akshaydeo merged commit 3bf3990 into dev May 25, 2026
14 checks passed
@akshaydeo
akshaydeo deleted the fix/bf-896-vk-blocked-models-editor-clean branch May 25, 2026 07:19

_, normalizedModel := schemas.ParseModelString(model, "")

for _, blocked := range blacklist {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Vaibhav701161 I merged this but can you create a follow up pr to replace this with slices.Contains?

akshaydeo pushed a commit that referenced this pull request May 25, 2026
## Summary

Refactors VK blocked-model matching to use `slices.Contains`, as requested in review.

This keeps the existing behavior unchanged while making the matching logic cleaner. Bare and provider-prefixed model names are still treated as equivalent, so entries like `mistral:latest` and `ollama/mistral:latest` continue to match correctly. Wildcard blocklists still block all models.

## Changes

* Added `blockedModelCandidates()` to build normalized match candidates for a model string.

  * Includes the lowercased raw model name.
  * Includes the lowercased bare model name after provider-prefix parsing.
* Updated `isModelBlockedByList()` to use `slices.Contains` for comparing normalized model forms.
* Preserved existing blocklist behavior for:

  * bare model vs bare request
  * prefixed blocklist entry vs bare request
  * bare blocklist entry vs prefixed request
  * prefixed model vs prefixed request
  * wildcard `["*"]`

Design decision:

* This is only a small internal refactor of the VK blocklist helper.
* No runtime behavior is intentionally changed.
* Provider-key behavior is unchanged.

## Type of change

* [ ] Bug fix
* [ ] Feature
* [x] Refactor
* [ ] Documentation
* [ ] Chore/CI

## Affected areas

* [ ] Core (Go)
* [ ] Transports (HTTP)
* [ ] Providers/Integrations
* [x] Plugins
* [ ] UI (React)
* [ ] Docs

## How to test

Sanity checks:

```sh
go test ./plugins/governance/...
go build -o ./tmp/bifrost-http ./transports/bifrost-http
```

Verified local Ollama E2E behavior:

* `["mistral:latest"]` + `mistral:latest` → `403 model_blocked`
* `["ollama/mistral:latest"]` + `mistral:latest` → `403 model_blocked`
* `["mistral:latest"]` + `ollama/mistral:latest` → `403 model_blocked`
* `["ollama/mistral:latest"]` + `ollama/mistral:latest` → `403 model_blocked`
* Different allowed model → `200 OK`
* Empty blocklist → `200 OK`
* Wildcard blocklist `["*"]` → all tested models blocked
* Same model in allowlist and blocklist → `403 model_blocked`

## Screenshots/Recordings

Not applicable. This PR only refactors backend governance matching logic.

## Breaking changes

* [ ] Yes
* [x] No

## Related issues

Follow-up to #3718

## Security considerations

This keeps VK blocked-model enforcement intact for both bare and provider-prefixed model strings.

No secrets, auth tokens, provider keys, or PII are exposed or stored by this change. Provider-key behavior is unchanged.

## 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
Vaibhav701161 added a commit that referenced this pull request May 26, 2026
## Summary

Adds the missing create/edit UI for VK-level blocked models and fixes runtime enforcement for provider-prefixed model names.

The previous VK blocked-model support had backend storage, API handling, runtime hooks, and read-only display, but the create/edit sheet did not expose the `Blocked Models` editor. This PR adds that missing UI field and also fixes an enforcement issue found during E2E testing: the UI stores selected models as `provider/model`, while the governance resolver could compare against bare model names. Because of that mismatch, blocked models could bypass the VK blacklist in some request paths.

## Changes

* Added editable `Blocked Models` field in the VK create/edit provider config section.

  * Placed between `Allowed Models` and `Allowed Keys`.
  * Uses the same `ModelMultiselect` pattern as the allowed models field.
  * Adds `blacklisted_models` to the zod schema.
  * Initializes edit mode with `config.blacklisted_models || []`.
  * Defaults new provider configs to `blacklisted_models: []`.

* Added prefix-aware blocked-model matching in governance.

  * Added `isModelBlockedByList()` to handle both bare and provider-prefixed model names.
  * This prevents bypasses where the UI stores `ollama/mistral:latest`, but the runtime request is evaluated as `mistral:latest`, or the other way around.

* Updated VK runtime enforcement paths to use the prefix-aware matcher:

  * `plugins/governance/resolver.go`
  * `plugins/governance/main.go`
  * `plugins/governance/utils.go`

Design decision:

* The fix keeps the existing `schemas.BlackList` behavior intact and adds a small VK-specific helper for prefix-aware comparison.
* Provider-key behavior is unchanged.

## Type of change

* [x] Bug fix
* [x] Feature
* [ ] Refactor
* [ ] Documentation
* [ ] Chore/CI

## Affected areas

* [ ] Core (Go)
* [ ] Transports (HTTP)
* [ ] Providers/Integrations
* [x] Plugins
* [x] UI (React)
* [ ] Docs

## How to test

Build checks:

```sh
go build -o ./tmp/bifrost-http ./transports/bifrost-http

cd ui
npm run build
```

Local server:

```sh
./tmp/bifrost-http -port 9090
```

Open:

```sh
http://localhost:9090/workspace/governance/virtual-keys
```

UI validation:

1. Open Governance → Virtual Keys.
2. Create or edit a virtual key.
3. Expand a provider config.
4. Confirm the field order is:

   * Allowed Models
   * Blocked Models
   * Allowed Keys
5. Select a blocked model and save.
6. Reopen the virtual key and confirm the blocked model is still selected.
7. Open the details sheet and confirm the blocked model is displayed.

E2E validation with local Ollama:

* Blocked bare model: `403 model_blocked`
* Blocked provider-prefixed model: `403 model_blocked`
* Allowed bare model: `200 OK`
* Allowed provider-prefixed model: `200 OK`
* Same model in allowlist and blocklist: `403 model_blocked`
* Empty blocklist: `200 OK`
* Wildcard blocklist `["*"]`: all tested models return `403 model_blocked`
* UI-created VK also works end-to-end with the same blocked/allowed behavior

## Screenshots/Recordings

Added/available recording showing the `Blocked Models` field in the VK provider config create/edit flow.



Uploading Screen Recording 2026-05-25 003101.mp4…




## Breaking changes

* [ ] Yes
* [x] No

## Related issues

BF-896

## Security considerations

This improves VK-level governance by ensuring blocked models are actually enforced for both bare and provider-prefixed model strings.

No secrets, provider keys, auth tokens, or PII are exposed or stored by this change. Provider-key behavior is unchanged.

## 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
akshaydeo pushed a commit that referenced this pull request May 26, 2026
## Summary

Restores the missing `Blocked Models` create/edit UI in the VK provider config sheet.

The backend enforcement (`isModelBlockedByList`, `blockedModelCandidates`, `blocklist_test.go`) was already present on `dev`. The only missing piece was the frontend editor, which became unreachable after dev was rebased/force-pushed following the original merge of #3718.

Changes in this PR:
- Added `blacklisted_models` to the zod provider config schema
- Initialized edit mode with `config.blacklisted_models || []`
- Added `blacklisted_models: []` default for new provider configs
- Added `Blocked Models` `ModelMultiselect` block, placed between `Allowed Models` and `Allowed Keys`
- Wildcard (`*`) toggle behavior consistent with allowed models

## How to test

Build:

```sh
go test ./plugins/governance/...
go build -o ./tmp/bifrost-http ./transports/bifrost-http
cd ui && npm run build
```

Manual UI check:

`http://localhost:9090/workspace/governance/virtual-keys`

Expected create/edit flow:

`Allowed Models → Blocked Models → Allowed Keys`

Runtime enforcement (prefix-aware, already on dev):
- blacklist `["ollama/mistral:latest"]`, request `"mistral:latest"` → 403
- blacklist `["mistral:latest"]`, request `"ollama/mistral:latest"` → 403
- wildcard `["*"]` → 403 for any model
- empty blacklist → passes through

## Related

Restores the UI lost from #3718. Backend enforcement already present on `dev`.
akshaydeo pushed a commit that referenced this pull request May 26, 2026
## Summary

Refactors VK blocked-model matching to use `slices.Contains`, as requested in review.

This keeps the existing behavior unchanged while making the matching logic cleaner. Bare and provider-prefixed model names are still treated as equivalent, so entries like `mistral:latest` and `ollama/mistral:latest` continue to match correctly. Wildcard blocklists still block all models.

## Changes

* Added `blockedModelCandidates()` to build normalized match candidates for a model string.

  * Includes the lowercased raw model name.
  * Includes the lowercased bare model name after provider-prefix parsing.
* Updated `isModelBlockedByList()` to use `slices.Contains` for comparing normalized model forms.
* Preserved existing blocklist behavior for:

  * bare model vs bare request
  * prefixed blocklist entry vs bare request
  * bare blocklist entry vs prefixed request
  * prefixed model vs prefixed request
  * wildcard `["*"]`

Design decision:

* This is only a small internal refactor of the VK blocklist helper.
* No runtime behavior is intentionally changed.
* Provider-key behavior is unchanged.

## Type of change

* [ ] Bug fix
* [ ] Feature
* [x] Refactor
* [ ] Documentation
* [ ] Chore/CI

## Affected areas

* [ ] Core (Go)
* [ ] Transports (HTTP)
* [ ] Providers/Integrations
* [x] Plugins
* [ ] UI (React)
* [ ] Docs

## How to test

Sanity checks:

```sh
go test ./plugins/governance/...
go build -o ./tmp/bifrost-http ./transports/bifrost-http
```

Verified local Ollama E2E behavior:

* `["mistral:latest"]` + `mistral:latest` → `403 model_blocked`
* `["ollama/mistral:latest"]` + `mistral:latest` → `403 model_blocked`
* `["mistral:latest"]` + `ollama/mistral:latest` → `403 model_blocked`
* `["ollama/mistral:latest"]` + `ollama/mistral:latest` → `403 model_blocked`
* Different allowed model → `200 OK`
* Empty blocklist → `200 OK`
* Wildcard blocklist `["*"]` → all tested models blocked
* Same model in allowlist and blocklist → `403 model_blocked`

## Screenshots/Recordings

Not applicable. This PR only refactors backend governance matching logic.

## Breaking changes

* [ ] Yes
* [x] No

## Related issues

Follow-up to #3718

## Security considerations

This keeps VK blocked-model enforcement intact for both bare and provider-prefixed model strings.

No secrets, auth tokens, provider keys, or PII are exposed or stored by this change. Provider-key behavior is unchanged.

## 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
akshaydeo pushed a commit that referenced this pull request May 26, 2026
## Summary

Restores the missing `Blocked Models` create/edit UI in the VK provider config sheet.

The backend enforcement (`isModelBlockedByList`, `blockedModelCandidates`, `blocklist_test.go`) was already present on `dev`. The only missing piece was the frontend editor, which became unreachable after dev was rebased/force-pushed following the original merge of #3718.

Changes in this PR:
- Added `blacklisted_models` to the zod provider config schema
- Initialized edit mode with `config.blacklisted_models || []`
- Added `blacklisted_models: []` default for new provider configs
- Added `Blocked Models` `ModelMultiselect` block, placed between `Allowed Models` and `Allowed Keys`
- Wildcard (`*`) toggle behavior consistent with allowed models

## How to test

Build:

```sh
go test ./plugins/governance/...
go build -o ./tmp/bifrost-http ./transports/bifrost-http
cd ui && npm run build
```

Manual UI check:

`http://localhost:9090/workspace/governance/virtual-keys`

Expected create/edit flow:

`Allowed Models → Blocked Models → Allowed Keys`

Runtime enforcement (prefix-aware, already on dev):
- blacklist `["ollama/mistral:latest"]`, request `"mistral:latest"` → 403
- blacklist `["mistral:latest"]`, request `"ollama/mistral:latest"` → 403
- wildcard `["*"]` → 403 for any model
- empty blacklist → passes through

## Related

Restores the UI lost from #3718. Backend enforcement already present on `dev`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants