Skip to content

refactor(governance): use slices for vk blocklist matching - #3727

Merged
akshaydeo merged 1 commit into
devfrom
fix/vk-blocked-models-use-slices-contains
May 25, 2026
Merged

akshaydeo merged 1 commit into
devfrom
fix/vk-blocked-models-use-slices-contains

Conversation

@Vaibhav701161

Copy link
Copy Markdown
Contributor

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
  • Refactor
  • Documentation
  • Chore/CI

Affected areas

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

How to test

Sanity checks:

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

Verified local Ollama E2E behavior:

  • ["mistral:latest"] + mistral:latest403 model_blocked
  • ["ollama/mistral:latest"] + mistral:latest403 model_blocked
  • ["mistral:latest"] + ollama/mistral:latest403 model_blocked
  • ["ollama/mistral:latest"] + ollama/mistral:latest403 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
  • 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

  • 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 25, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 3a9f8d00-0884-480b-a5e6-59a3f51b4dfe

📥 Commits

Reviewing files that changed from the base of the PR and between bd29273 and dbf95a4.

📒 Files selected for processing (4)
  • plugins/governance/blocklist_test.go
  • plugins/governance/main.go
  • plugins/governance/resolver.go
  • plugins/governance/utils.go

📝 Walkthrough

Summary by CodeRabbit

  • Bug Fixes

    • Improved blacklist matching to use normalized, case-insensitive comparisons so blocked models are detected consistently across provider selection and model-allowance checks, including when models use different identifier formats (prefixed vs bare).
  • Tests

    • Added unit tests covering wildcard blocking, exact matches, prefixed vs bare name cross-matching, empty lists, and case-insensitive behavior.

Walkthrough

Refactors blacklist matching to generate raw and normalized model candidates, compares candidates case-insensitively using slices.ContainsFunc, replaces prior IsBlocked calls across governance call sites, and adds unit tests covering wildcard, exact, prefix, and case-insensitive matches.

Changes

Model Blacklist Matching Refactor

Layer / File(s) Summary
Candidate helper and tests
plugins/governance/utils.go, plugins/governance/blocklist_test.go
Adds slices import and blockedModelCandidates that returns raw and normalized model forms when different; refactors isModelBlockedByList to compare candidate overlap case-insensitively; adds TestIsModelBlockedByList covering empty, wildcard, exact, prefix, and case-insensitive matches.
Call-site blacklist checks update
plugins/governance/utils.go, plugins/governance/main.go, plugins/governance/resolver.go
Replaces BlacklistedModels.IsBlocked(model) calls with isModelBlockedByList(BlacklistedModels, model) in filterModelsForVirtualKey, loadBalanceProvider pre-filter, and (*BudgetResolver).isModelAllowed blacklist pass.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • maximhq/bifrost#3718: Makes related changes to blocked-model matching and enforcement in governance code.
  • maximhq/bifrost#3653: Modifies governance blocked-model decision path and touches related blacklist evaluation logic.

Suggested reviewers

  • akshaydeo
  • danpiths
  • roroghost17

Poem

🐰 I hop through candidates, raw and neat,
Parsing names so matches meet,
Case-folded checks and slices' call,
I catch the blocks and trim them all,
A tidy blacklist — springtime treat.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.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 clearly summarizes the main refactoring change: moving blocklist matching logic to use slices instead of manual comparison.
Description check ✅ Passed The description is comprehensive and follows the template structure with all key sections completed: summary, changes, type of change, affected areas, testing, breaking changes, related issues, and security considerations.
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/vk-blocked-models-use-slices-contains

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.

@greptile-apps

greptile-apps Bot commented May 25, 2026

Copy link
Copy Markdown
Contributor

Confidence Score: 5/5

Safe to merge; the refactor is well-contained within the governance plugin and all call sites retain their provider guard.

All three call sites preserve the existing provider-matching guard before invoking the new helper, so the blast radius of any edge-case mismatch is narrow. The only gap found is that blocklist entries with uppercase provider prefixes won't have their prefix stripped by ParseModelString and therefore won't match bare model strings — a pre-existing limitation of the case-sensitive provider registry that doesn't affect the typical lowercase-provider workflow.

plugins/governance/utils.go — the blockedModelCandidates helper and its interaction with ParseModelString's case-sensitive provider lookup.

Important Files Changed

Filename Overview
plugins/governance/utils.go Adds blockedModelCandidates and isModelBlockedByList; case-sensitive provider lookup in ParseModelString means entries with uppercase provider prefixes won't have the prefix stripped when matching bare model names.
plugins/governance/blocklist_test.go New table-driven tests covering empty blocklist, wildcard, bare/prefixed permutations, and case-insensitive matching; missing a case for uppercase-prefixed blocklist entry vs bare model.
plugins/governance/resolver.go Swaps IsBlocked call for the new isModelBlockedByList helper; provider guard is preserved upstream, no behavior change at the call site.
plugins/governance/main.go Same mechanical swap of IsBlocked to isModelBlockedByList; straightforward and correct.

Reviews (4): Last reviewed commit: "refactor(governance): use slices for vk ..." | Re-trigger Greptile

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 25, 2026
@Vaibhav701161
Vaibhav701161 force-pushed the fix/vk-blocked-models-use-slices-contains branch from 5b443ae to bd29273 Compare May 25, 2026 08:56

@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

🤖 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 `@plugins/governance/utils.go`:
- Around line 91-94: The blacklist matching in blockedModelCandidates (and the
later comparison) currently lowercases inputs and uses exact slices.Contains
which breaks the original case-insensitive semantics of
schemas.BlackList.Contains; update the comparisons to use case-insensitive
equality (strings.EqualFold) instead of plain lowercase+slices.Contains — e.g.,
when checking against schemas.BlackList entries use slices.ContainsFunc or an
explicit loop that calls strings.EqualFold for model vs. blacklist entry; keep
the normalization via schemas.ParseModelString but perform final membership
tests with strings.EqualFold to preserve original semantics.
🪄 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: 6ca2df17-8c3f-4718-ba8a-0d2777fbd22b

📥 Commits

Reviewing files that changed from the base of the PR and between 5b443ae and bd29273.

📒 Files selected for processing (4)
  • plugins/governance/blocklist_test.go
  • plugins/governance/main.go
  • plugins/governance/resolver.go
  • plugins/governance/utils.go

Comment thread plugins/governance/utils.go Outdated
Signed-off-by: Vaibhav mittal <vaibhavmittal929@gmail.com>
@Vaibhav701161
Vaibhav701161 force-pushed the fix/vk-blocked-models-use-slices-contains branch from bd29273 to 5b443ae Compare May 25, 2026 11:49
@CLAassistant

CLAassistant commented May 25, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

coderabbitai[bot]
coderabbitai Bot previously approved these changes May 25, 2026
@Vaibhav701161
Vaibhav701161 force-pushed the fix/vk-blocked-models-use-slices-contains branch from 5b443ae to dbf95a4 Compare May 25, 2026 11:53

akshaydeo commented May 25, 2026

Copy link
Copy Markdown
Contributor

Merge activity

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

@akshaydeo
akshaydeo merged commit 151a61b into dev May 25, 2026
14 checks passed
@akshaydeo
akshaydeo deleted the fix/vk-blocked-models-use-slices-contains branch May 25, 2026 16:36
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 akshaydeo mentioned this pull request May 26, 2026
akshaydeo added a commit that referenced this pull request May 26, 2026
## ✨ Features

- **Azure v1 API Migration** — Migrated Azure provider to the v1 API:
removed the `api-version` query parameter and the
`/openai/deployments/{model}/...` URL pattern in favor of
`/openai/v1/{operation}`; the `api_version` field has been dropped from
`AzureKeyConfig` (#3661, #3756)
- **EnvVar Support for OTEL & Prometheus Configs** — `CollectorURL`,
`MetricsEndpoint`, headers, push gateway URL, and basic auth credentials
can now be sourced from environment variables (e.g.,
`env.OTEL_COLLECTOR_URL`); added a new `ConfigMarshallerPlugin`
interface that lets plugins control storage/redaction round-trips
(#3651)
- **OTel Extra Header Forwarding** — `x-bf-eh-*` extra headers forwarded
to upstream providers are now also emitted on the request span under
`gen_ai.request.extra_header.*` for end-to-end tracing (#3730)
- **OTel Semantic Conventions** — Aligned OTel attribute keys with the
OpenTelemetry GenAI spec (canonical `gen_ai.*` and new `bifrost.*`
attributes); legacy attributes are retained in parallel to avoid
breaking existing dashboards (#3732)
- **VK Quota with Provider Configs** — `GetVirtualKeyQuotaByValue` and
the `getVirtualKeyQuota` HTTP response now include `provider_configs`
with their budgets and rate limits (#3721)
- **MCP Temp Token Non-Auth Toggle** — Added
`mcp_enable_temp_token_auth` client config flag to gate short-lived MCP
token minting for non-authenticated users (#3720)
- **Responses Stream in JSON Parser** — `jsonparser` plugin now handles
OpenAI Responses API streaming (`ResponsesStreamRequest`) in addition to
chat completions (#3749)
- **Session API Rework** — Logout now calls both the password-based
session logout and OAuth logout endpoints and resets all RTK Query cache
state (#3698)

## 🐞 Fixed

- **Streaming Latency for Observability** — Deferred root span
termination to the trace completer callback for streaming requests so
request latency is no longer inflated by header-flush time (#3762)
- **Stream Cancellation Race** — Set `BifrostContextKeyConnectionClosed`
before closing the stream and short-circuit `idleTimeoutReader.Read`
when the connection is already closed to avoid panics and hangs on
cancellation (#3733)
- **Bedrock Cache Points** — Strip cache points from Bedrock requests
for models that do not support prompt caching (e.g., GLM, Llama) to
avoid Converse API errors (#3754)
- **Bedrock Empty Text Blocks** — Skip empty/nil text blocks during
Bedrock response conversion to avoid invalid messages (#3747)
- **Bedrock Reasoning + Tools** — Preserve reasoning content blocks on
assistant turns that also contain tool calls in the Bedrock chat
converter (#3690)
- **Bedrock Search Content & Video** — Restored search content and video
parts that were being dropped from Bedrock-native passthrough requests
(#3729)
- **Structured Output Stop Reason** — Fixed an incorrect `tool_calls`
finish reason when structured output is combined with extended-thinking
tools (#3685)
- **Gemini Tool Schema Passthrough** — Forward full tool parameter
schemas via `parametersJsonSchema` instead of the lossy `parameters`
form; corrected tool response role to `user`; resolved structured output
+ tools conflict (#3761)
- **Anthropic Stop Reason & Tool Versions** — Normalized stop reason
mapping (`end_turn` to `stop`, `tool_use` to `tool_calls`, `max_tokens`
to `length`) and upgraded `text_editor_20250124`/`str_replace_editor` to
`text_editor_20250728` for computer-use tools (#3761)
- **Azure Endpoint Redaction** — Fixed a panic when
`AzureKeyConfig.Endpoint` is a literal value rather than an env
reference (#3761)
- **Auth Middleware Path Match** — Match temp-token auth middleware
whitelist against the request path only, not the full URI with query
parameters (#3737)
- **Governance Blocked Models UI** — Restored the missing Blocked Models
create/edit UI in the VK provider config sheet (#3750)
- **Logging Plugin Cleanup Drain** — Fixed a shutdown race where
`batchWriter` could drop in-flight log entries; `Cleanup` now drains
both the recovered batch and remaining queue within a 30-second budget
(#3717)
- **Model Rankings Empty Entries** — Excluded entries with empty `model`
values from model rankings matview queries so blank rows no longer
surface in the UI (#3758)
- **User Filter Duplicates** — Recreated `mv_filter_users` matview to
require non-empty `user_name`, eliminating duplicate filter dropdown
entries (#3764)
- **User Filter Display Name** — Use `user_name` instead of `user_id` as
the display label for users in logging filters (#3691)
- **Large Numeric ID Precision** — Preserve large numeric IDs in URL
search params by skipping JSON parsing for plain strings (#3692)

## 🔧 Refactors & Chores

- **Error Propagation for GetAvailable\* APIs** — `GetAvailable*`
methods on `LoggerPlugin`/`LogManager` now return wrapped errors instead
of silently logging and returning empty slices (#3759)
- **Governance Blocklist Matching** — Use `slices.Contains` for VK
blocked-model matching for clearer code with identical semantics (#3727)
- **Exported `ResolvePeriod`** — Renamed `resolvePeriod` to
`ResolvePeriod` so external packages can reuse the period parsing
(#3763)

## 📚 Docs

- **OTEL Env Var Documentation** — Documented `env.VAR_NAME` support for
`collector_url`, `metrics_endpoint`, and headers in OTEL/Prometheus
plugin docs
- **OTEL OSS Features & Examples** — Added OTEL documentation to the OSS
features list with usage examples (#3731)
- **Anthropic Auth Recommendation** — Recommend `ANTHROPIC_AUTH_TOKEN`
over `ANTHROPIC_CUSTOM_HEADERS` for Claude Code authentication (#3686)
@akshaydeo akshaydeo mentioned this pull request May 27, 2026
18 tasks
akshaydeo added a commit that referenced this pull request May 27, 2026
## Summary

This PR releases Bifrost OSS `v1.5.5` and Enterprise `v1.4.4`, bumping all module pins from `v1.5.12`/`v1.3.12` to `v1.5.13`/`v1.3.13` across core, framework, and all plugins. It also hardens the Docker manifest shell scripts, expands CI egress allowlists, and updates documentation to reflect the new SCIM-based user provisioning feature.

## Changes

- **Module version bumps**: All `go.mod`/`go.sum` files updated from `core v1.5.12` → `v1.5.13`, `framework v1.3.12` → `v1.3.13`, and all plugin versions incremented accordingly (`compat`, `governance`, `jsonparser`, `logging`, `maxim`, `mocker`, `otel`, `prompts`, `semanticcache`, `telemetry`).
- **Docker manifest scripts**: Added `#!/usr/bin/env bash` shebang and `set -euo pipefail` to `create-docker-manifest.sh` and `create-docker-manifest-ubi9.sh`; quoted all variable expansions and switched `jq -r` to `jq -er` to fail on null digests.
- **CI egress allowlist**: Added `production.cloudfront.docker.com:443` to Docker-related job allowlists, and added `_https._tcp.dl.google.com:443` and `motd.ubuntu.com:443` to the Ubuntu package job allowlist.
- **Changelog files**: Cleared per-module `changelog.md` files (content moved into the new versioned docs). Added `docs/changelogs/v1.5.5.mdx` and `docs/changelogs/ent-v1.4.4.mdx` with full release notes, and registered both in `docs/docs.json`.
- **Documentation**: Replaced the SSO Integration link with a User Provisioning (SCIM) link in both `README.md` and `transports/README.md`.
- **Enterprise v1.4.4 highlights** (documented): Kafka and Google Cloud Pub/Sub observability sinks, chunked streaming with a 100 MB inter-node message ceiling, BigQuery custom labels via env vars using the new `ConfigMarshallerPlugin` interface, temporary access token expiry extensions, and a multi-node cluster integration harness.
- **OSS v1.5.5 highlights** (documented): Azure v1 API migration, env-var support for OTel/Prometheus configs, OTel extra-header forwarding and semantic-convention alignment, virtual key quota including provider configs, Responses API streaming in `jsonparser`, and a batch of Bedrock, Gemini, Anthropic, Azure, and logging plugin fixes.

## Type of change

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

## Affected areas

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

## How to test

```sh
# Core/Transports
go version
go test ./...

# Verify Docker manifest scripts exit on error
bash -n .github/workflows/scripts/create-docker-manifest.sh
bash -n .github/workflows/scripts/create-docker-manifest-ubi9.sh
```

Validate that the new changelog pages (`changelogs/v1.5.5` and `changelogs/ent-v1.4.4`) render correctly in the docs site.

## Screenshots/Recordings

N/A

## Breaking changes

- [x] Yes
- [ ] No

The Azure provider no longer accepts `api_version` in `AzureKeyConfig` and has migrated to the `/openai/v1/{operation}` URL pattern. See the [v1.4.0 Migration Guide](https://docs.getbifrost.ai/enterprise/migration-guides/v1.4.0) for full details.

## Related issues

#3661, #3756, #3651, #3730, #3732, #3754, #3747, #3690, #3729, #3685, #3733, #3761, #3735, #3721, #3720, #3749, #3698, #3762, #3750, #3727, #3717, #3759, #3758, #3764, #3691, #3692, #3737, #3763

## Security considerations

- The `ConfigMarshallerPlugin` interface redacts secrets (OTel collector URLs, Prometheus push gateway credentials, BigQuery labels) at config storage time and rehydrates them at load time, preventing plaintext secret persistence.
- Docker manifest scripts now use `set -euo pipefail`, preventing silent failures that could result in malformed or missing image manifests being pushed.

## Checklist

- [x] I read `docs/contributing/README.md` and followed the guidelines
- [x] I added/updated tests where appropriate
- [x] I updated documentation where needed
- [x] I verified builds succeed (Go and UI)
- [x] I verified the CI pipeline passes locally if applicable
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 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 maximhq#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
akhsaul pushed a commit to akhsaul/bifrost that referenced this pull request Aug 27, 2026
## ✨ Features

- **Azure v1 API Migration** — Migrated Azure provider to the v1 API:
removed the `api-version` query parameter and the
`/openai/deployments/{model}/...` URL pattern in favor of
`/openai/v1/{operation}`; the `api_version` field has been dropped from
`AzureKeyConfig` (maximhq#3661, maximhq#3756)
- **EnvVar Support for OTEL & Prometheus Configs** — `CollectorURL`,
`MetricsEndpoint`, headers, push gateway URL, and basic auth credentials
can now be sourced from environment variables (e.g.,
`env.OTEL_COLLECTOR_URL`); added a new `ConfigMarshallerPlugin`
interface that lets plugins control storage/redaction round-trips
(maximhq#3651)
- **OTel Extra Header Forwarding** — `x-bf-eh-*` extra headers forwarded
to upstream providers are now also emitted on the request span under
`gen_ai.request.extra_header.*` for end-to-end tracing (maximhq#3730)
- **OTel Semantic Conventions** — Aligned OTel attribute keys with the
OpenTelemetry GenAI spec (canonical `gen_ai.*` and new `bifrost.*`
attributes); legacy attributes are retained in parallel to avoid
breaking existing dashboards (maximhq#3732)
- **VK Quota with Provider Configs** — `GetVirtualKeyQuotaByValue` and
the `getVirtualKeyQuota` HTTP response now include `provider_configs`
with their budgets and rate limits (maximhq#3721)
- **MCP Temp Token Non-Auth Toggle** — Added
`mcp_enable_temp_token_auth` client config flag to gate short-lived MCP
token minting for non-authenticated users (maximhq#3720)
- **Responses Stream in JSON Parser** — `jsonparser` plugin now handles
OpenAI Responses API streaming (`ResponsesStreamRequest`) in addition to
chat completions (maximhq#3749)
- **Session API Rework** — Logout now calls both the password-based
session logout and OAuth logout endpoints and resets all RTK Query cache
state (maximhq#3698)

## 🐞 Fixed

- **Streaming Latency for Observability** — Deferred root span
termination to the trace completer callback for streaming requests so
request latency is no longer inflated by header-flush time (maximhq#3762)
- **Stream Cancellation Race** — Set `BifrostContextKeyConnectionClosed`
before closing the stream and short-circuit `idleTimeoutReader.Read`
when the connection is already closed to avoid panics and hangs on
cancellation (maximhq#3733)
- **Bedrock Cache Points** — Strip cache points from Bedrock requests
for models that do not support prompt caching (e.g., GLM, Llama) to
avoid Converse API errors (maximhq#3754)
- **Bedrock Empty Text Blocks** — Skip empty/nil text blocks during
Bedrock response conversion to avoid invalid messages (maximhq#3747)
- **Bedrock Reasoning + Tools** — Preserve reasoning content blocks on
assistant turns that also contain tool calls in the Bedrock chat
converter (maximhq#3690)
- **Bedrock Search Content & Video** — Restored search content and video
parts that were being dropped from Bedrock-native passthrough requests
(maximhq#3729)
- **Structured Output Stop Reason** — Fixed an incorrect `tool_calls`
finish reason when structured output is combined with extended-thinking
tools (maximhq#3685)
- **Gemini Tool Schema Passthrough** — Forward full tool parameter
schemas via `parametersJsonSchema` instead of the lossy `parameters`
form; corrected tool response role to `user`; resolved structured output
+ tools conflict (maximhq#3761)
- **Anthropic Stop Reason & Tool Versions** — Normalized stop reason
mapping (`end_turn` to `stop`, `tool_use` to `tool_calls`, `max_tokens`
to `length`) and upgraded `text_editor_20250124`/`str_replace_editor` to
`text_editor_20250728` for computer-use tools (maximhq#3761)
- **Azure Endpoint Redaction** — Fixed a panic when
`AzureKeyConfig.Endpoint` is a literal value rather than an env
reference (maximhq#3761)
- **Auth Middleware Path Match** — Match temp-token auth middleware
whitelist against the request path only, not the full URI with query
parameters (maximhq#3737)
- **Governance Blocked Models UI** — Restored the missing Blocked Models
create/edit UI in the VK provider config sheet (maximhq#3750)
- **Logging Plugin Cleanup Drain** — Fixed a shutdown race where
`batchWriter` could drop in-flight log entries; `Cleanup` now drains
both the recovered batch and remaining queue within a 30-second budget
(maximhq#3717)
- **Model Rankings Empty Entries** — Excluded entries with empty `model`
values from model rankings matview queries so blank rows no longer
surface in the UI (maximhq#3758)
- **User Filter Duplicates** — Recreated `mv_filter_users` matview to
require non-empty `user_name`, eliminating duplicate filter dropdown
entries (maximhq#3764)
- **User Filter Display Name** — Use `user_name` instead of `user_id` as
the display label for users in logging filters (maximhq#3691)
- **Large Numeric ID Precision** — Preserve large numeric IDs in URL
search params by skipping JSON parsing for plain strings (maximhq#3692)

## 🔧 Refactors & Chores

- **Error Propagation for GetAvailable\* APIs** — `GetAvailable*`
methods on `LoggerPlugin`/`LogManager` now return wrapped errors instead
of silently logging and returning empty slices (maximhq#3759)
- **Governance Blocklist Matching** — Use `slices.Contains` for VK
blocked-model matching for clearer code with identical semantics (maximhq#3727)
- **Exported `ResolvePeriod`** — Renamed `resolvePeriod` to
`ResolvePeriod` so external packages can reuse the period parsing
(maximhq#3763)

## 📚 Docs

- **OTEL Env Var Documentation** — Documented `env.VAR_NAME` support for
`collector_url`, `metrics_endpoint`, and headers in OTEL/Prometheus
plugin docs
- **OTEL OSS Features & Examples** — Added OTEL documentation to the OSS
features list with usage examples (maximhq#3731)
- **Anthropic Auth Recommendation** — Recommend `ANTHROPIC_AUTH_TOKEN`
over `ANTHROPIC_CUSTOM_HEADERS` for Claude Code authentication (maximhq#3686)
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 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 maximhq#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
occcat pushed a commit to occcat/bifrost that referenced this pull request Sep 2, 2026
## ✨ Features

- **Azure v1 API Migration** — Migrated Azure provider to the v1 API:
removed the `api-version` query parameter and the
`/openai/deployments/{model}/...` URL pattern in favor of
`/openai/v1/{operation}`; the `api_version` field has been dropped from
`AzureKeyConfig` (maximhq#3661, maximhq#3756)
- **EnvVar Support for OTEL & Prometheus Configs** — `CollectorURL`,
`MetricsEndpoint`, headers, push gateway URL, and basic auth credentials
can now be sourced from environment variables (e.g.,
`env.OTEL_COLLECTOR_URL`); added a new `ConfigMarshallerPlugin`
interface that lets plugins control storage/redaction round-trips
(maximhq#3651)
- **OTel Extra Header Forwarding** — `x-bf-eh-*` extra headers forwarded
to upstream providers are now also emitted on the request span under
`gen_ai.request.extra_header.*` for end-to-end tracing (maximhq#3730)
- **OTel Semantic Conventions** — Aligned OTel attribute keys with the
OpenTelemetry GenAI spec (canonical `gen_ai.*` and new `bifrost.*`
attributes); legacy attributes are retained in parallel to avoid
breaking existing dashboards (maximhq#3732)
- **VK Quota with Provider Configs** — `GetVirtualKeyQuotaByValue` and
the `getVirtualKeyQuota` HTTP response now include `provider_configs`
with their budgets and rate limits (maximhq#3721)
- **MCP Temp Token Non-Auth Toggle** — Added
`mcp_enable_temp_token_auth` client config flag to gate short-lived MCP
token minting for non-authenticated users (maximhq#3720)
- **Responses Stream in JSON Parser** — `jsonparser` plugin now handles
OpenAI Responses API streaming (`ResponsesStreamRequest`) in addition to
chat completions (maximhq#3749)
- **Session API Rework** — Logout now calls both the password-based
session logout and OAuth logout endpoints and resets all RTK Query cache
state (maximhq#3698)

## 🐞 Fixed

- **Streaming Latency for Observability** — Deferred root span
termination to the trace completer callback for streaming requests so
request latency is no longer inflated by header-flush time (maximhq#3762)
- **Stream Cancellation Race** — Set `BifrostContextKeyConnectionClosed`
before closing the stream and short-circuit `idleTimeoutReader.Read`
when the connection is already closed to avoid panics and hangs on
cancellation (maximhq#3733)
- **Bedrock Cache Points** — Strip cache points from Bedrock requests
for models that do not support prompt caching (e.g., GLM, Llama) to
avoid Converse API errors (maximhq#3754)
- **Bedrock Empty Text Blocks** — Skip empty/nil text blocks during
Bedrock response conversion to avoid invalid messages (maximhq#3747)
- **Bedrock Reasoning + Tools** — Preserve reasoning content blocks on
assistant turns that also contain tool calls in the Bedrock chat
converter (maximhq#3690)
- **Bedrock Search Content & Video** — Restored search content and video
parts that were being dropped from Bedrock-native passthrough requests
(maximhq#3729)
- **Structured Output Stop Reason** — Fixed an incorrect `tool_calls`
finish reason when structured output is combined with extended-thinking
tools (maximhq#3685)
- **Gemini Tool Schema Passthrough** — Forward full tool parameter
schemas via `parametersJsonSchema` instead of the lossy `parameters`
form; corrected tool response role to `user`; resolved structured output
+ tools conflict (maximhq#3761)
- **Anthropic Stop Reason & Tool Versions** — Normalized stop reason
mapping (`end_turn` to `stop`, `tool_use` to `tool_calls`, `max_tokens`
to `length`) and upgraded `text_editor_20250124`/`str_replace_editor` to
`text_editor_20250728` for computer-use tools (maximhq#3761)
- **Azure Endpoint Redaction** — Fixed a panic when
`AzureKeyConfig.Endpoint` is a literal value rather than an env
reference (maximhq#3761)
- **Auth Middleware Path Match** — Match temp-token auth middleware
whitelist against the request path only, not the full URI with query
parameters (maximhq#3737)
- **Governance Blocked Models UI** — Restored the missing Blocked Models
create/edit UI in the VK provider config sheet (maximhq#3750)
- **Logging Plugin Cleanup Drain** — Fixed a shutdown race where
`batchWriter` could drop in-flight log entries; `Cleanup` now drains
both the recovered batch and remaining queue within a 30-second budget
(maximhq#3717)
- **Model Rankings Empty Entries** — Excluded entries with empty `model`
values from model rankings matview queries so blank rows no longer
surface in the UI (maximhq#3758)
- **User Filter Duplicates** — Recreated `mv_filter_users` matview to
require non-empty `user_name`, eliminating duplicate filter dropdown
entries (maximhq#3764)
- **User Filter Display Name** — Use `user_name` instead of `user_id` as
the display label for users in logging filters (maximhq#3691)
- **Large Numeric ID Precision** — Preserve large numeric IDs in URL
search params by skipping JSON parsing for plain strings (maximhq#3692)

## 🔧 Refactors & Chores

- **Error Propagation for GetAvailable\* APIs** — `GetAvailable*`
methods on `LoggerPlugin`/`LogManager` now return wrapped errors instead
of silently logging and returning empty slices (maximhq#3759)
- **Governance Blocklist Matching** — Use `slices.Contains` for VK
blocked-model matching for clearer code with identical semantics (maximhq#3727)
- **Exported `ResolvePeriod`** — Renamed `resolvePeriod` to
`ResolvePeriod` so external packages can reuse the period parsing
(maximhq#3763)

## 📚 Docs

- **OTEL Env Var Documentation** — Documented `env.VAR_NAME` support for
`collector_url`, `metrics_endpoint`, and headers in OTEL/Prometheus
plugin docs
- **OTEL OSS Features & Examples** — Added OTEL documentation to the OSS
features list with usage examples (maximhq#3731)
- **Anthropic Auth Recommendation** — Recommend `ANTHROPIC_AUTH_TOKEN`
over `ANTHROPIC_CUSTOM_HEADERS` for Claude Code authentication (maximhq#3686)
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.

3 participants