mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-24 16:41:30 +02:00
[proxy] Let model discovery past the provider allowlist
The guardrail enforces its own per-provider model allowlist and fails closed when the request names no model, which is right for a path-routed inference request whose shape the parser could not read. GET /v1/models names no model anywhere, so discovery still denied with model_unknown for exactly the accounts that configured an allowlist — the case skipping the management pre-flight was meant to fix. Only one of the two gates had been opened, and a client reads the 403 as an empty model picker. Exempt requests the router marked non-inference from the unknown-model branch. A named model is still checked, so the exemption covers only the endpoints that genuinely name nothing: the listing and the warm probe.
This commit is contained in:
@@ -85,8 +85,9 @@ func (m *Middleware) Invoke(_ context.Context, in *middleware.Input) (*middlewar
|
||||
model, modelPresent := lookupMetadata(in.Metadata, middleware.KeyLLMModel)
|
||||
providerID, _ := lookupMetadata(in.Metadata, middleware.KeyLLMResolvedProviderID)
|
||||
surface, _ := lookupMetadata(in.Metadata, middleware.KeyLLMProvider)
|
||||
nonInference, _ := lookupMetadata(in.Metadata, middleware.KeyLLMNonInference)
|
||||
|
||||
if denial := m.evaluateAllowlist(providerID, surface, model, modelPresent); denial != nil {
|
||||
if denial := m.evaluateAllowlist(providerID, surface, model, modelPresent, nonInference == "true"); denial != nil {
|
||||
return denial, nil
|
||||
}
|
||||
|
||||
@@ -115,7 +116,7 @@ func (m *Middleware) Close() error { return nil }
|
||||
// evaluateAllowlist denies when the resolved provider's allowlist rejects the
|
||||
// model; nil means proceed. Scoped to the provider llm_router resolved, so an
|
||||
// unrestricted provider (absent from config) is never caught by another's list.
|
||||
func (m *Middleware) evaluateAllowlist(providerID, surface, model string, modelPresent bool) *middleware.Output {
|
||||
func (m *Middleware) evaluateAllowlist(providerID, surface, model string, modelPresent, nonInference bool) *middleware.Output {
|
||||
if len(m.cfg.ProviderAllowlists) == 0 {
|
||||
return nil
|
||||
}
|
||||
@@ -134,7 +135,18 @@ func (m *Middleware) evaluateAllowlist(providerID, surface, model string, modelP
|
||||
// Fail closed: with an allowlist in effect for this provider, a request whose
|
||||
// model the parser couldn't extract (absent/empty) is denied. This enforces
|
||||
// the allowlist for path-routed providers (Bedrock, Vertex) with no body model.
|
||||
//
|
||||
// The exception is a non-inference endpoint the router already authorised.
|
||||
// The model listing and the connection-warming probe name no model
|
||||
// anywhere — not in a body, not in the path — so failing closed here
|
||||
// rejected model discovery for exactly the accounts that configured an
|
||||
// allowlist, which is the outage this endpoint is meant to avoid. The
|
||||
// per-model lookup does name one (the router stamps it from the path), so
|
||||
// it still falls through to the allowlist check below.
|
||||
if !modelPresent || normaliseModel(model) == "" {
|
||||
if nonInference {
|
||||
return nil
|
||||
}
|
||||
return denyModel(surface, "", denyCodeModelUnknown, denyMessageModelUnknown, denyReasonModelUnknown)
|
||||
}
|
||||
if modelInAllowlist(allowlist, model) {
|
||||
|
||||
@@ -343,3 +343,52 @@ func TestFactoryNormalisesAllowlist(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, middleware.DecisionAllow, out2.Decision, "trimmed entry must still match")
|
||||
}
|
||||
|
||||
// TestAllowlistSkipsNonInferenceWithoutModel covers the reported regression:
|
||||
// GET /v1/models carries no model anywhere, so the fail-closed rule above
|
||||
// denied model discovery for exactly the accounts that configured a provider
|
||||
// allowlist — the clients that read a 403 here render an empty model picker.
|
||||
// The router authorises those endpoints by path before the guardrail sees
|
||||
// them, so an absent model there is expected rather than undeterminable.
|
||||
func TestAllowlistSkipsNonInferenceWithoutModel(t *testing.T) {
|
||||
mw := New(providerCfg("gpt-4o"))
|
||||
out, err := mw.Invoke(context.Background(), newInputProvider(testProvider,
|
||||
middleware.KV{Key: middleware.KeyLLMNonInference, Value: "true"},
|
||||
))
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, out)
|
||||
assert.Equal(t, middleware.DecisionAllow, out.Decision,
|
||||
"model discovery must not be refused because it names no model")
|
||||
}
|
||||
|
||||
// TestAllowlistStillAppliesToNonInferenceWithModel pins that the exemption is
|
||||
// scoped to requests that genuinely name nothing. The per-model lookup
|
||||
// (GET /v1/models/{id}) is non-inference too, but the router stamps the model
|
||||
// from its path, so the allowlist must still decide it — otherwise the
|
||||
// exemption becomes a way to confirm a model the policy blocks.
|
||||
func TestAllowlistStillAppliesToNonInferenceWithModel(t *testing.T) {
|
||||
mw := New(providerCfg("gpt-4o"))
|
||||
|
||||
t.Run("model in the allowlist", func(t *testing.T) {
|
||||
out, err := mw.Invoke(context.Background(), newInputProvider(testProvider,
|
||||
middleware.KV{Key: middleware.KeyLLMNonInference, Value: "true"},
|
||||
middleware.KV{Key: middleware.KeyLLMModel, Value: "gpt-4o"},
|
||||
))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, middleware.DecisionAllow, out.Decision,
|
||||
"an allowlisted model must stay reachable")
|
||||
})
|
||||
|
||||
t.Run("model outside the allowlist", func(t *testing.T) {
|
||||
out, err := mw.Invoke(context.Background(), newInputProvider(testProvider,
|
||||
middleware.KV{Key: middleware.KeyLLMNonInference, Value: "true"},
|
||||
middleware.KV{Key: middleware.KeyLLMModel, Value: "claude-opus-5"},
|
||||
))
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, middleware.DecisionDeny, out.Decision,
|
||||
"non-inference must not become a way past the allowlist")
|
||||
require.NotNil(t, out.DenyReason)
|
||||
assert.Equal(t, "llm_policy.model_blocked", out.DenyReason.Code,
|
||||
"a named but blocked model is blocked, not unknown")
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user