mirror of
https://github.com/netbirdio/netbird.git
synced 2026-08-30 11:31:29 +02:00
[management,client] 0.74.5 branch sync (#6769)
## Describe your changes * [proxy] enforce model allowlist for URL-routed providers (Bedrock/Vertex) by @mlsmaycon in https://github.com/netbirdio/netbird/pull/6764 * [management] Remove proxy peer stale deduplication logic by @mlsmaycon in https://github.com/netbirdio/netbird/pull/6768 ## Issue ticket number and link ## Stack <!-- branch-stack --> ### Checklist - [ ] Is it a bug fix - [ ] Is a typo/documentation fix - [ ] Is a feature enhancement - [ ] It is a refactor - [ ] Created tests that fail without the change (if possible) - [ ] This change does **not** modify the public API, gRPC protocols, functionality behavior, CLI / service flags, or introduce a new feature — **OR** I have discussed it with the NetBird team beforehand (link the issue / Slack thread in the description). See [CONTRIBUTING.md](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTING.md#discuss-changes-with-the-netbird-team-first). > By submitting this pull request, you confirm that you have read and agree to the terms of the [Contributor License Agreement](https://github.com/netbirdio/netbird/blob/main/CONTRIBUTOR_LICENSE_AGREEMENT.md). ## Documentation Select exactly one: - [ ] I added/updated documentation for this change - [x] Documentation is **not needed** for this change (explain why) ### Docs PR URL (required if "docs added" is checked) Paste the PR link from https://github.com/netbirdio/docs here: https://github.com/netbirdio/docs/pull/__ <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Added model-allowlist guardrails for path-routed providers, including Bedrock and Vertex. - Added Bedrock request support for chat interactions. - Added guardrail management capabilities. - **Bug Fixes** - Requests with missing or blank model identifiers are now denied when a model allowlist is configured, improving fail-closed protection. - Corrected provider-specific request handling and session tracking for Bedrock interactions. - **Tests** - Expanded coverage for allowlist enforcement and provider routing scenarios. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Theodor Midtlien <theodor@midtlien.com> Co-authored-by: blaugrau90 <61945343+blaugrau90@users.noreply.github.com> Co-authored-by: Viktor Liu <17948409+lixmal@users.noreply.github.com>
This commit is contained in:
@@ -25,6 +25,14 @@ const (
|
||||
denyCodeModel = "llm_policy.model_blocked"
|
||||
denyReasonModel = "model_blocked"
|
||||
denyMessageModel = "model is not in the policy allowlist"
|
||||
// Deny reason used when an allowlist is configured but the request model
|
||||
// could not be determined. URL/path-routed providers (AWS Bedrock, Google
|
||||
// Vertex, ...) carry the model outside the JSON body, so a request shape the
|
||||
// parser does not recognise reaches the guardrail with no model. Such a
|
||||
// request must be denied (fail closed), never waved through.
|
||||
denyCodeModelUnknown = "llm_policy.model_unknown"
|
||||
denyReasonModelUnknown = "model_unknown"
|
||||
denyMessageModelUnknown = "request model could not be determined for the policy allowlist"
|
||||
)
|
||||
|
||||
// Middleware enforces the model allowlist and optionally captures the
|
||||
@@ -108,23 +116,37 @@ func (m *Middleware) evaluateAllowlist(model string, modelPresent bool) *middlew
|
||||
if len(m.cfg.ModelAllowlist) == 0 {
|
||||
return nil
|
||||
}
|
||||
if !modelPresent {
|
||||
return nil
|
||||
// Fail closed: with an allowlist configured, a request whose model the
|
||||
// upstream parser could not extract (absent or empty) must be denied rather
|
||||
// than allowed. This is what enforces the allowlist for URL/path-routed
|
||||
// providers (Bedrock, Vertex, ...) whose model lives outside the JSON body.
|
||||
if !modelPresent || normaliseModel(model) == "" {
|
||||
return denyModel("", denyCodeModelUnknown, denyMessageModelUnknown, denyReasonModelUnknown)
|
||||
}
|
||||
if m.modelInAllowlist(model) {
|
||||
return nil
|
||||
}
|
||||
return denyModel(model, denyCodeModel, denyMessageModel, denyReasonModel)
|
||||
}
|
||||
|
||||
// denyModel builds a 403 deny Output for a model-allowlist rejection. model is
|
||||
// included in the details only when non-empty.
|
||||
func denyModel(model, code, message, reason string) *middleware.Output {
|
||||
details := map[string]string{}
|
||||
if model != "" {
|
||||
details["model"] = model
|
||||
}
|
||||
return &middleware.Output{
|
||||
Decision: middleware.DecisionDeny,
|
||||
DenyStatus: 403,
|
||||
DenyReason: &middleware.DenyReason{
|
||||
Code: denyCodeModel,
|
||||
Message: denyMessageModel,
|
||||
Details: map[string]string{"model": model},
|
||||
Code: code,
|
||||
Message: message,
|
||||
Details: details,
|
||||
},
|
||||
Metadata: []middleware.KV{
|
||||
{Key: middleware.KeyLLMPolicyDecision, Value: "deny"},
|
||||
{Key: middleware.KeyLLMPolicyReason, Value: denyReasonModel},
|
||||
{Key: middleware.KeyLLMPolicyReason, Value: reason},
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
@@ -102,13 +102,44 @@ func TestAllowlistCaseInsensitive(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestAllowlistMissingModelKeyAllows(t *testing.T) {
|
||||
func TestAllowlistMissingModelKeyDenies(t *testing.T) {
|
||||
// Fail closed: with an allowlist configured, a request whose model the
|
||||
// parser could not extract (URL/path-routed providers such as Bedrock or
|
||||
// Vertex whose shape wasn't recognised) must be denied, not allowed.
|
||||
mw := New(Config{ModelAllowlist: []string{"gpt-4o"}})
|
||||
out, err := mw.Invoke(context.Background(), newInput())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, middleware.DecisionAllow, out.Decision, "missing model key must allow even with non-empty allowlist")
|
||||
require.NotNil(t, out)
|
||||
assert.Equal(t, middleware.DecisionDeny, out.Decision, "absent model must be denied when an allowlist is set")
|
||||
assert.Equal(t, 403, out.DenyStatus, "deny status must be 403")
|
||||
require.NotNil(t, out.DenyReason, "deny reason must be populated")
|
||||
assert.Equal(t, "llm_policy.model_unknown", out.DenyReason.Code, "deny code must be model_unknown")
|
||||
dec, _ := metaValue(t, out.Metadata, middleware.KeyLLMPolicyDecision)
|
||||
assert.Equal(t, "allow", dec, "decision must be allow when model key is absent")
|
||||
assert.Equal(t, "deny", dec, "decision must be deny when model key is absent")
|
||||
reason, _ := metaValue(t, out.Metadata, middleware.KeyLLMPolicyReason)
|
||||
assert.Equal(t, "model_unknown", reason, "reason metadata must be model_unknown")
|
||||
}
|
||||
|
||||
func TestAllowlistEmptyModelValueDenies(t *testing.T) {
|
||||
// A present-but-empty model is as undeterminable as an absent one.
|
||||
mw := New(Config{ModelAllowlist: []string{"gpt-4o"}})
|
||||
out, err := mw.Invoke(context.Background(), newInput(
|
||||
middleware.KV{Key: middleware.KeyLLMModel, Value: " "},
|
||||
))
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, out)
|
||||
assert.Equal(t, middleware.DecisionDeny, out.Decision, "empty model must be denied when an allowlist is set")
|
||||
require.NotNil(t, out.DenyReason, "deny reason must be populated")
|
||||
assert.Equal(t, "llm_policy.model_unknown", out.DenyReason.Code, "deny code must be model_unknown")
|
||||
}
|
||||
|
||||
func TestAllowlistEmptyListAllowsMissingModel(t *testing.T) {
|
||||
// Without an allowlist there is nothing to enforce, so a missing model is
|
||||
// still allowed — the fail-closed rule only applies when a list is set.
|
||||
mw := New(Config{})
|
||||
out, err := mw.Invoke(context.Background(), newInput())
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, middleware.DecisionAllow, out.Decision, "no allowlist must allow even without a model")
|
||||
}
|
||||
|
||||
func TestPromptCaptureDisabledEmitsNoPrompt(t *testing.T) {
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
package llm_request_parser
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/netbirdio/netbird/proxy/internal/middleware"
|
||||
"github.com/netbirdio/netbird/proxy/internal/middleware/builtin/llm_guardrail"
|
||||
)
|
||||
|
||||
// runParserGuardrail runs the request parser then the model-allowlist guardrail
|
||||
// in SlotOnRequest order, threading the parser's metadata into the guardrail the
|
||||
// same way the real chain does. It returns the guardrail decision so tests can
|
||||
// assert allowlist enforcement for URL/path-routed providers end to end.
|
||||
func runParserGuardrail(t *testing.T, url string, body []byte, allowlist []string) *middleware.Output {
|
||||
t.Helper()
|
||||
parser := newMiddleware(t)
|
||||
parsed, err := parser.Invoke(context.Background(), &middleware.Input{
|
||||
Slot: middleware.SlotOnRequest,
|
||||
URL: url,
|
||||
Body: body,
|
||||
})
|
||||
require.NoError(t, err, "parser must not error")
|
||||
|
||||
guard := llm_guardrail.New(llm_guardrail.Config{ModelAllowlist: allowlist})
|
||||
out, err := guard.Invoke(context.Background(), &middleware.Input{
|
||||
Slot: middleware.SlotOnRequest,
|
||||
Metadata: parsed.Metadata,
|
||||
})
|
||||
require.NoError(t, err, "guardrail must not error")
|
||||
require.NotNil(t, out, "guardrail must return an output")
|
||||
return out
|
||||
}
|
||||
|
||||
// TestModelAllowlist_URLRoutedProviders validates that the model allowlist is
|
||||
// enforced for providers whose model travels in the URL path (AWS Bedrock,
|
||||
// Google Vertex) rather than the JSON body. The "unknown action" case is the
|
||||
// regression guard for #6751: a Bedrock request shape the parser cannot map to a
|
||||
// model must fail closed under an allowlist instead of bypassing it.
|
||||
func TestModelAllowlist_URLRoutedProviders(t *testing.T) {
|
||||
const bedrockBody = `{"anthropic_version":"bedrock-2023-05-31","messages":[{"role":"user","content":"hi"}]}`
|
||||
const vertexBody = `{"anthropic_version":"vertex-2023-10-16","messages":[{"role":"user","content":"hi"}]}`
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
url string
|
||||
body string
|
||||
allowlist []string
|
||||
decision middleware.Decision
|
||||
denyCode string
|
||||
}{
|
||||
{
|
||||
name: "bedrock allowed model passes",
|
||||
url: "https://bedrock-runtime.us-east-1.amazonaws.com/model/us.anthropic.claude-haiku-4-5-v1:0/invoke",
|
||||
body: bedrockBody,
|
||||
allowlist: []string{"anthropic.claude-haiku-4-5"},
|
||||
decision: middleware.DecisionAllow,
|
||||
},
|
||||
{
|
||||
name: "bedrock disallowed model denied",
|
||||
url: "https://bedrock-runtime.us-east-1.amazonaws.com/model/us.anthropic.claude-opus-4-8-v1:0/invoke",
|
||||
body: bedrockBody,
|
||||
allowlist: []string{"anthropic.claude-haiku-4-5"},
|
||||
decision: middleware.DecisionDeny,
|
||||
denyCode: "llm_policy.model_blocked",
|
||||
},
|
||||
{
|
||||
name: "bedrock unknown action fails closed",
|
||||
url: "https://bedrock-runtime.us-east-1.amazonaws.com/model/us.anthropic.claude-opus-4-8-v1:0/some-future-action",
|
||||
body: bedrockBody,
|
||||
allowlist: []string{"anthropic.claude-haiku-4-5"},
|
||||
decision: middleware.DecisionDeny,
|
||||
denyCode: "llm_policy.model_unknown",
|
||||
},
|
||||
{
|
||||
name: "vertex disallowed model denied",
|
||||
url: "/v1/projects/p/locations/global/publishers/anthropic/models/claude-opus-4-8@20250101:rawPredict",
|
||||
body: vertexBody,
|
||||
allowlist: []string{"claude-haiku-4-5"},
|
||||
decision: middleware.DecisionDeny,
|
||||
denyCode: "llm_policy.model_blocked",
|
||||
},
|
||||
{
|
||||
name: "vertex allowed model passes",
|
||||
url: "/v1/projects/p/locations/global/publishers/anthropic/models/claude-haiku-4-5@20250101:rawPredict",
|
||||
body: vertexBody,
|
||||
allowlist: []string{"claude-haiku-4-5"},
|
||||
decision: middleware.DecisionAllow,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
out := runParserGuardrail(t, tt.url, []byte(tt.body), tt.allowlist)
|
||||
assert.Equal(t, tt.decision, out.Decision, "unexpected decision for %s", tt.name)
|
||||
if tt.decision == middleware.DecisionDeny {
|
||||
require.NotNil(t, out.DenyReason, "deny reason must be set for %s", tt.name)
|
||||
assert.Equal(t, 403, out.DenyStatus, "deny status must be 403 for %s", tt.name)
|
||||
assert.Equal(t, tt.denyCode, out.DenyReason.Code, "deny code for %s", tt.name)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user