fix(modelcatalog): wildcard allow-list permits any model on custom providers

[mickgvirtu]

Summary

With a virtual-key allow-list of ["*"] ("All models"), IsModelAllowedForProvider still cross-checks the model against the catalog via GetProvidersForModel. For a custom (OpenAI-compatible) provider the catalog can't enumerate the backend's models, so legitimate models the operator pointed the provider at are wrongly denied with model_blocked.

Changes

• For a custom provider, ["*"] now means "all models on this provider" (returns true). Native providers stay catalog-cross-checked, so a wildcard can't spray a model to a provider that doesn't serve it.
• Removed a stale doc comment describing the old list-models-disabled fast path.
• Safety: this runs after the governance blacklist (BlacklistedModels.IsBlocked, which wins over the allow-list), so a wildcard does not bypass explicit denials.

Type of change

• Bug fix

Affected areas

• Core (Go)

How to test

go test ./framework/modelcatalog/

New models_test.go covers: wildcard+custom+uncatalogued ⇒ allow; wildcard+native+unserved ⇒ deny; empty ⇒ deny; explicit match.

[CLAassistant]

All committers have signed the CLA.

[coderabbitai]

📝 Walkthrough

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Updated model allowance behavior for wildcard ("*") allow-lists: custom providers now treat wildcard as allowing any model unconditionally, while native providers still require the model to be listed in the catalog as served.

• Tests

  • Added coverage for allow/deny cases, including wildcard vs empty allow-lists and native vs custom provider behavior.

• Documentation

  • Clarified the documented wildcard ("*") behavior for native and custom providers.

Walkthrough

IsModelAllowedForProvider in framework/modelcatalog/models.go is updated so that a wildcard allow-list ("*") on a custom provider unconditionally allows all models, replacing the prior check that gated on whether ListModels was disabled. The documentation is clarified to distinguish this behavior from native providers, which remain subject to catalog lookup. A new test file pins five allow/deny scenarios for this function.

Changes

Custom provider wildcard allow-list behavior and test coverage

Layer / File(s)	Summary
Wildcard allow-list logic for custom providers framework/modelcatalog/models.go	Removes the hasListModelsEndpointDisabled condition for custom providers under a wildcard allow-list; any providerConfig with a non-nil CustomProviderConfig now unconditionally passes. Native providers still require GetProvidersForModel(model) membership. Updated comment documents this behavior.
Documentation of wildcard behavior distinction docs/architecture/framework/model-catalog.mdx	Updates the IsModelAllowedForProvider documentation to clarify that wildcard ("*") behavior differs between custom providers (unconditional allowance) and native providers (catalog-determined via GetProvidersForModel).
Test helpers and imports framework/modelcatalog/models_test.go	Adds the test file with required imports, then introduces emptyCatalog() and customProviderConfig() helpers that enable testing without pre-populating catalog data.
Test scenarios for allow/deny logic framework/modelcatalog/models_test.go	Adds five test functions covering wildcard-on-custom-provider (allows uncatalogued models), wildcard-on-native-unserved (denies), wildcard-on-native-served (allows), empty-allow-list (denies both provider types), and explicit-match (permits only listed model names).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• maximhq/bifrost#4124: Directly modifies the same IsModelAllowedForProvider wildcard (allowedModels.IsUnrestricted() / "*") path in framework/modelcatalog/models.go, introducing the catalog-opaque logic for custom providers that this PR further refines.

Suggested reviewers

• akshaydeo
• danpiths

Poem

🐇 Hop, hop, the wildcard's free,
No ListModels gate to bother me!
Custom providers skip the check,
While native ones still dot the spec.
Five tests now guard the den—
The catalog bunny grins again! 🌟

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: fixing wildcard allow-list behavior for custom providers in the model catalog.
Description check	✅ Passed	The description covers Summary, Changes, Type of change, Affected areas, and How to test sections; all required information is present and well-explained.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.
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 unit tests (beta)

• Create PR with unit tests

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[greptile-apps]

Confidence Score: 5/5

Safe to merge — the change is minimal, well-tested, and the governance blacklist is confirmed to run before this allow-list check in all three call sites.

The logic change is one condition removed from a single function. All callers in the governance plugin do a blacklist check before calling IsModelAllowedForProvider, so a wildcard can never bypass an explicit denial. Native providers are unchanged and remain catalog-gated. The new test file directly pins every behavior branch modified by this PR.

No files require special attention.

Important Files Changed

Filename	Overview
framework/modelcatalog/models.go	Removed the hasListModelsEndpointDisabled gate; wildcard now returns true for any custom provider. Security invariants (blacklist wins, native providers still catalog-checked) are intact and confirmed by caller inspection in governance plugins.
framework/modelcatalog/models_test.go	New test file with 5 focused cases covering wildcard+custom (allow), wildcard+native unserved (deny), wildcard+native served (allow), empty deny-by-default, and explicit list match/no-match. Covers all branches of the changed logic path.
docs/architecture/framework/model-catalog.mdx	Doc comment and behavior description updated accurately to reflect the native/custom wildcard asymmetry. No discrepancies with the code change.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[IsModelAllowedForProvider called] --> B{allowedModels.IsUnrestricted?}
    B -->|No| C{allowedModels.IsEmpty?}
    C -->|Yes| D[return false — deny-by-default]
    C -->|No| E[Iterate explicit list — direct or provider-prefixed match]
    E --> F[return true/false]

    B -->|Yes — wildcard| G{isCustomProvider?}
    G -->|Yes — CustomProviderConfig != nil| H[return true — all models permitted on custom backend]
    G -->|No — native provider| I[GetProvidersForModel model]
    I --> J{provider in result?}
    J -->|Yes| K[return true]
    J -->|No| L[return false]

    subgraph Callers governance plugins
        M[Pass 1: BlacklistedModels.IsBlocked?] -->|Yes| N[skip — blacklist wins]
        M -->|No| O[Pass 2: IsModelAllowedForProvider]
    end

Loading

%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
    A[IsModelAllowedForProvider called] --> B{allowedModels.IsUnrestricted?}
    B -->|No| C{allowedModels.IsEmpty?}
    C -->|Yes| D[return false — deny-by-default]
    C -->|No| E[Iterate explicit list — direct or provider-prefixed match]
    E --> F[return true/false]

    B -->|Yes — wildcard| G{isCustomProvider?}
    G -->|Yes — CustomProviderConfig != nil| H[return true — all models permitted on custom backend]
    G -->|No — native provider| I[GetProvidersForModel model]
    I --> J{provider in result?}
    J -->|Yes| K[return true]
    J -->|No| L[return false]

    subgraph Callers governance plugins
        M[Pass 1: BlacklistedModels.IsBlocked?] -->|Yes| N[skip — blacklist wins]
        M -->|No| O[Pass 2: IsModelAllowedForProvider]
    end

Loading

_{Reviews (2): Last reviewed commit: "modelcatalog: wildcard allow-list permit..." | Re-trigger Greptile}

[coderabbitai]

🧹 Nitpick comments (1)

framework/modelcatalog/models_test.go (1)

45-55: ⚡ Quick win

Add the positive native wildcard case.

This only proves wildcard denies unserved native models. Since the PR preserves native catalog cross-checking, add a seeded native-provider model case that must return true; otherwise, a regression that denies all native wildcard requests would still pass.

🤖 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 `@framework/modelcatalog/models_test.go` around lines 45 - 55, The test
TestIsModelAllowedForProvider_WildcardNativeProviderUnserved only validates the
negative case where a wildcard denies an unserved model. Add a complementary
positive test case that seeds the catalog with a model served by a native
provider (such as OpenAI) and verifies that IsModelAllowedForProvider returns
true when called with a wildcard for that same provider and model. This ensures
the code correctly allows wildcards for native providers when models are
properly served, preventing regressions where all native wildcard requests might
be incorrectly denied.

🤖 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 `@framework/modelcatalog/models_test.go`:
- Around line 45-55: The test
TestIsModelAllowedForProvider_WildcardNativeProviderUnserved only validates the
negative case where a wildcard denies an unserved model. Add a complementary
positive test case that seeds the catalog with a model served by a native
provider (such as OpenAI) and verifies that IsModelAllowedForProvider returns
true when called with a wildcard for that same provider and model. This ensures
the code correctly allows wildcards for native providers when models are
properly served, preventing regressions where all native wildcard requests might
be incorrectly denied.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 794c8382-74f6-4e2b-8a5e-4548a3c66474

📥 Commits

Reviewing files that changed from the base of the PR and between 96bb2bd and ef70510.

📒 Files selected for processing (2)

• framework/modelcatalog/models.go
• framework/modelcatalog/models_test.go

[mickgvirtu]

Thanks — both addressed:

• Positive native-wildcard case (coderabbit): added TestIsModelAllowedForProvider_WildcardNativeProviderServed — seeds a native provider (OpenAI) with gpt-4o via the live store and asserts a wildcard allows it. Guards against a regression that denied all native wildcard requests.
• Stale docs (greptile): updated docs/architecture/framework/model-catalog.mdx — the wildcard behavior section + inline example now describe the custom-vs-native asymmetry (custom ⇒ allow-all; native ⇒ catalog cross-check) instead of "always delegates to the catalog".

go test ./framework/modelcatalog/ passes (custom-allow, native-served-allow, native-unserved-deny, empty-deny, explicit-match).

[coderabbitai]

Caution

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

⚠️ Outside diff range comments (1)

docs/architecture/framework/model-catalog.mdx (1)

292-298: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix example call signature to match implementation

At Line 294, the docs call IsModelAllowedForProvider with 3 args, but the current method takes 4 args (providerConfig before allowedModels). This example is currently copy/paste incorrect.

Suggested docs patch

 isAllowed := modelCatalog.IsModelAllowedForProvider(
     schemas.OpenRouter,
     "gpt-4o",
-    schemas.WhiteList{"*"}, // wildcard = check catalog
+    nil, // providerConfig (nil for native provider example)
+    schemas.WhiteList{"*"}, // wildcard = check catalog
 )

As per coding guidelines, docs under docs/** should maintain parity with code behavior/contracts.

🤖 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 `@docs/architecture/framework/model-catalog.mdx` around lines 292 - 298, The
example call to IsModelAllowedForProvider in the documentation is incorrect - it
currently passes 3 arguments but the actual method signature requires 4
arguments with providerConfig appearing before allowedModels. Update the example
code snippet to include the missing providerConfig argument and reorder the
parameters to match the actual method implementation signature for
IsModelAllowedForProvider.

Source: Coding guidelines

🤖 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 `@docs/architecture/framework/model-catalog.mdx`:
- Around line 292-298: The example call to IsModelAllowedForProvider in the
documentation is incorrect - it currently passes 3 arguments but the actual
method signature requires 4 arguments with providerConfig appearing before
allowedModels. Update the example code snippet to include the missing
providerConfig argument and reorder the parameters to match the actual method
implementation signature for IsModelAllowedForProvider.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 4f7156ed-9cbc-4e2b-9552-fc299a258d9e

📥 Commits

Reviewing files that changed from the base of the PR and between ef70510 and 3e052a2.

📒 Files selected for processing (3)

• docs/architecture/framework/model-catalog.mdx
• framework/modelcatalog/models.go
• framework/modelcatalog/models_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• framework/modelcatalog/models.go