From 1796b2a1d8119bdaf30ec6f1b41aaf45e74460bf Mon Sep 17 00:00:00 2001 From: mlsmaycon Date: Tue, 11 Aug 2026 15:20:18 +0000 Subject: [PATCH] [proxy] Let model discovery past the provider allowlist MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../builtin/llm_guardrail/middleware.go | 16 +++++- .../builtin/llm_guardrail/middleware_test.go | 49 +++++++++++++++++++ 2 files changed, 63 insertions(+), 2 deletions(-) diff --git a/proxy/internal/middleware/builtin/llm_guardrail/middleware.go b/proxy/internal/middleware/builtin/llm_guardrail/middleware.go index ddf710d24..d2b14f265 100644 --- a/proxy/internal/middleware/builtin/llm_guardrail/middleware.go +++ b/proxy/internal/middleware/builtin/llm_guardrail/middleware.go @@ -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) { diff --git a/proxy/internal/middleware/builtin/llm_guardrail/middleware_test.go b/proxy/internal/middleware/builtin/llm_guardrail/middleware_test.go index 5f35fefd3..19d8473fe 100644 --- a/proxy/internal/middleware/builtin/llm_guardrail/middleware_test.go +++ b/proxy/internal/middleware/builtin/llm_guardrail/middleware_test.go @@ -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") + }) +}