diff --git a/docs/architecture/framework/model-catalog.mdx b/docs/architecture/framework/model-catalog.mdx index 73b73a0d54..a159ae8867 100644 --- a/docs/architecture/framework/model-catalog.mdx +++ b/docs/architecture/framework/model-catalog.mdx @@ -289,7 +289,8 @@ providers := modelCatalog.GetProvidersForModel("claude-3-5-sonnet") Validate if a model is allowed for a specific provider based on an allowed models list. This method is used internally by governance and load balancing plugins. ```go -// ["*"] wildcard - uses catalog to determine support +// ["*"] wildcard on a native provider - uses catalog to determine support +// (custom providers allow any model with a wildcard) isAllowed := modelCatalog.IsModelAllowedForProvider( schemas.OpenRouter, "gpt-4o", @@ -316,7 +317,7 @@ isAllowed := modelCatalog.IsModelAllowedForProvider( **Behavior**: -- **`["*"]` wildcard**: Delegates to `GetProvidersForModel` (includes cross-provider logic) - this is the "allow all via catalog" mode +- **`["*"]` wildcard**: For **custom** (OpenAI-compatible) providers, allows any model unconditionally — the catalog can't enumerate a custom backend's models. For **native** providers, delegates to `GetProvidersForModel` (includes cross-provider logic) so a wildcard still only permits models the catalog knows the provider serves. - **Non-empty explicit list**: Checks for both direct matches and provider-prefixed entries - **Empty slice (`[]string{}` / empty `schemas.WhiteList`)**: Returns `false` (deny-all) - mirrors the config deny-by-default semantics diff --git a/framework/modelcatalog/models.go b/framework/modelcatalog/models.go index b28454669c..780e8ede61 100644 --- a/framework/modelcatalog/models.go +++ b/framework/modelcatalog/models.go @@ -196,21 +196,21 @@ func (mc *ModelCatalog) GetProvidersForModel(model string) []schemas.ModelProvid // provider given an explicit allowedModels list (used by VK governance // checks, not by the static keyconfig allow set). // -// - allowedModels=["*"]: defer to GetProvidersForModel (with custom-provider -// fast path when list-models is disabled). +// - allowedModels=["*"]: custom providers are allowed unconditionally (the catalog +// can't enumerate a custom backend's models); native providers are cross-checked +// against the catalog via GetProvidersForModel. // - allowedModels=[]: deny-by-default. // - explicit allowedModels: direct or provider-prefixed match against the // provider's catalog. func (mc *ModelCatalog) IsModelAllowedForProvider(provider schemas.ModelProvider, model string, providerConfig *configstore.ProviderConfig, allowedModels schemas.WhiteList) bool { - isCustomProvider := false - hasListModelsEndpointDisabled := false - if providerConfig != nil && providerConfig.CustomProviderConfig != nil { - isCustomProvider = true - hasListModelsEndpointDisabled = !providerConfig.CustomProviderConfig.IsOperationAllowed(schemas.ListModelsRequest) - } + isCustomProvider := providerConfig != nil && providerConfig.CustomProviderConfig != nil if allowedModels.IsUnrestricted() { - if isCustomProvider && hasListModelsEndpointDisabled { + // A custom provider serves whatever its backend exposes, which the catalog can't fully + // enumerate even with list-models on, so a wildcard means "all models on this provider" + // (the operator's GUI selection), not just catalog-known ones. Native providers stay + // catalog-cross-checked so a wildcard can't spray a model to providers that don't serve it. + if isCustomProvider { return true } return slices.Contains(mc.GetProvidersForModel(model), provider) diff --git a/framework/modelcatalog/models_test.go b/framework/modelcatalog/models_test.go new file mode 100644 index 0000000000..5067ba3481 --- /dev/null +++ b/framework/modelcatalog/models_test.go @@ -0,0 +1,92 @@ +package modelcatalog + +import ( + "testing" + + bifrost "github.com/maximhq/bifrost/core" + "github.com/maximhq/bifrost/core/schemas" + "github.com/maximhq/bifrost/framework/configstore" + "github.com/maximhq/bifrost/framework/modelcatalog/datasheet" + "github.com/maximhq/bifrost/framework/modelcatalog/keyconfig" + "github.com/maximhq/bifrost/framework/modelcatalog/live" +) + +// emptyCatalog builds a ModelCatalog with empty (but valid) backing stores, so +// GetProvidersForModel resolves to "no provider serves this model" — enough to +// exercise the wildcard allow/deny logic without seeding a datasheet. +func emptyCatalog() *ModelCatalog { + logger := bifrost.NewNoOpLogger() + return &ModelCatalog{ + logger: logger, + datasheet: datasheet.New(nil, logger, datasheet.Config{}), + live: live.New(logger), + keyconf: keyconfig.New(logger), + } +} + +func customProviderConfig() *configstore.ProviderConfig { + return &configstore.ProviderConfig{ + CustomProviderConfig: &schemas.CustomProviderConfig{}, + } +} + +// TestIsModelAllowedForProvider_WildcardCustomProvider pins the patched behavior: a ["*"] +// allow-list permits ANY model on a custom provider, including one the catalog has never heard +// of (an empty catalog here) — fixing the spurious model_blocked 403 on internal models. +func TestIsModelAllowedForProvider_WildcardCustomProvider(t *testing.T) { + mc := emptyCatalog() + allowed := mc.IsModelAllowedForProvider( + schemas.ModelProvider("my-custom"), "glm-4.6", customProviderConfig(), schemas.WhiteList{"*"}) + if !allowed { + t.Fatal("wildcard on a custom provider must allow an uncatalogued model") + } +} + +// TestIsModelAllowedForProvider_WildcardNativeProviderUnserved pins the asymmetry: a wildcard on +// a NATIVE provider is still catalog-cross-checked, so a model no provider serves is denied (a +// wildcard must not spray a model to a provider that doesn't serve it). +func TestIsModelAllowedForProvider_WildcardNativeProviderUnserved(t *testing.T) { + mc := emptyCatalog() + allowed := mc.IsModelAllowedForProvider( + schemas.OpenAI, "model-no-one-serves", nil, schemas.WhiteList{"*"}) + if allowed { + t.Fatal("wildcard on a native provider must still be catalog-cross-checked (deny when unserved)") + } +} + +// TestIsModelAllowedForProvider_WildcardNativeProviderServed is the positive native case: a +// wildcard on a native provider that DOES serve the model (per the catalog) is allowed. Guards +// against a regression that denied all native wildcard requests. +func TestIsModelAllowedForProvider_WildcardNativeProviderServed(t *testing.T) { + mc := emptyCatalog() + mc.live.Upsert(schemas.OpenAI, "test-key", false, []string{"gpt-4o"}) + allowed := mc.IsModelAllowedForProvider( + schemas.OpenAI, "gpt-4o", nil, schemas.WhiteList{"*"}) + if !allowed { + t.Fatal("wildcard on a native provider that serves the model must be allowed") + } +} + +// TestIsModelAllowedForProvider_EmptyDenies pins deny-by-default: an empty allow-list denies +// regardless of provider type. +func TestIsModelAllowedForProvider_EmptyDenies(t *testing.T) { + mc := emptyCatalog() + if mc.IsModelAllowedForProvider(schemas.ModelProvider("my-custom"), "glm-4.6", customProviderConfig(), schemas.WhiteList{}) { + t.Fatal("empty allow-list must deny on a custom provider") + } + if mc.IsModelAllowedForProvider(schemas.OpenAI, "gpt-4o", nil, schemas.WhiteList{}) { + t.Fatal("empty allow-list must deny on a native provider") + } +} + +// TestIsModelAllowedForProvider_ExplicitMatch pins explicit allow-listing: a model named in the +// list is allowed (direct match) without needing the catalog. +func TestIsModelAllowedForProvider_ExplicitMatch(t *testing.T) { + mc := emptyCatalog() + if !mc.IsModelAllowedForProvider(schemas.ModelProvider("my-custom"), "glm-4.6", customProviderConfig(), schemas.WhiteList{"glm-4.6"}) { + t.Fatal("explicitly allow-listed model must be allowed") + } + if mc.IsModelAllowedForProvider(schemas.ModelProvider("my-custom"), "other", customProviderConfig(), schemas.WhiteList{"glm-4.6"}) { + t.Fatal("a model not in the explicit allow-list must be denied") + } +}