agentnetwork: tighten comment blocks to <=250 chars

Condense the comments added in this change so each block stays within 250
characters (and well under 140 per line), keeping the essential rationale.
No behaviour change.

e2e/agentnetwork/guardrail_multipolicy_test.go

@@ -14,40 +14,9 @@ import (
 	"github.com/netbirdio/netbird/shared/management/http/api"
 )
 
-// TestGuardrailMultiPolicyModelAllowlist is the end-to-end regression guard for
+// TestGuardrailMultiPolicyModelAllowlist: modelSelected served (200), grpOther's
-// the multi-policy interactions of the model-allowlist guardrail: what happens
+// modelOther denied for the grpMain client (403 model_blocked, no cross-group
-// when several enabled policies govern an account and some carry a guardrail
+// leak), and openModel on the un-guardrailed policy's provider served (200).
-// while others don't. The earlier implementation merged every policy's allowlist
-// into ONE account-wide union enforced flat on every request, which produced two
-// defects this test pins:
-//
-//   - false-ALLOW (cross-group leak): a model allowlisted only for another
-//     group's policy became usable by any caller, because the union ignored
-//     which policy/group actually authorised the request; and
-//   - false-DENY: a policy with NO guardrail (intended unrestricted) still had
-//     its traffic blocked by some other policy's allowlist.
-//
-// The fix scopes enforcement to the matched policy/group in management
-// (SelectPolicyForRequest) with a per-provider fail-closed backstop at the
-// proxy. The account here has one client in grpMain and three policies over two
-// catch-all upstreams (the mock vLLM answers any model 200, so a 403 can only be
-// a policy decision, never a routing miss):
-//
-//   - polMain : grpMain  -> pRestricted, guardrail allowlisting modelSelected
-//   - polOther: grpOther -> pRestricted, guardrail allowlisting modelOther
-//   - polOpen : grpMain  -> pOpen,       NO guardrail (unrestricted)
-//
-// Providers declare their models so routing is deterministic. Over the tunnel,
-// as the grpMain client:
-//
-//   - modelSelected on pRestricted is served (200) — allowed by grpMain's policy;
-//   - modelOther on pRestricted is denied 403 (llm_policy.model_blocked) — it is
-//     allowlisted only for grpOther and must NOT leak to grpMain; and
-//   - openModel on pOpen is served (200) — the un-guardrailed policy leaves that
-//     provider unrestricted and must NOT be blocked by another policy's list.
-//
-// Under the old account-wide union the middle case returned 200 (the leak) and
-// the last case returned 403 (the false-deny); both are inverted here.
 func TestGuardrailMultiPolicyModelAllowlist(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Minute)
 	defer cancel()

management/internals/modules/agentnetwork/manager.go

@@ -92,13 +92,10 @@ type PolicySelectionInput struct {
 	UserID     string
 	GroupIDs   []string
 	ProviderID string
-	// Model is the (already-normalised) upstream model identifier the proxy
+	// Model is the already-normalised upstream model id the proxy extracted
-	// extracted from the request. The proxy's request parser strips
+	// (parser strips Bedrock region/version, Vertex @version), so a
-	// path-routed provider decoration (Bedrock region/version, Vertex
+	// case-insensitive compare suffices. Empty = undetermined → not permitted
-	// @version) before emitting it, so a plain case-insensitive compare
+	// (fail closed).
-	// against a guardrail allowlist is sufficient here. Empty when the model
-	// could not be determined — treated as "not permitted" by any policy that
-	// restricts models (fail closed), mirroring the proxy guardrail.
 	Model string
 }
 

management/internals/modules/agentnetwork/policyselect.go

@@ -37,9 +37,8 @@ const (
 	//nolint:gosec // account deny code label, not a credential
 	denyCodeAccountBudgetCapExceeded = "llm_account.budget_cap_exceeded"
 	// denyCodeModelBlocked is returned when policies govern the request's
-	// (provider, caller-groups) but none of them permits the requested model
+	// (provider, caller-groups) but none permits the model. Matches the proxy
-	// under its guardrail allowlist. Matches the proxy guardrail's code so the
+	// guardrail's code so both layers surface the same label.
-	// two enforcement layers surface the same label.
 	denyCodeModelBlocked = "llm_policy.model_blocked"
 )
 
@@ -165,18 +164,9 @@ func (m *managerImpl) SelectPolicyForRequest(ctx context.Context, in PolicySelec
 	}
 	candidates := filterApplicablePolicies(policies, in)
 
-	// Model-allowlist gate (per-policy, per-group). Among the policies that
+	// Model-allowlist gate scoped to the matched policies: keep candidates whose
-	// authorise this (provider, caller-groups), keep only those whose guardrails
+	// guardrails permit the model (none enabled = unrestricted), deny when
-	// permit the requested model; a policy with no allowlist-enabled guardrail is
+	// policies apply but none permits it. Skip the load when none has a guardrail.
-	// unrestricted and always permits. When policies govern this request but none
-	// permits the model, deny. This is the authoritative allowlist decision:
-	// because it is scoped to the matched policies, a model allowlisted for one
-	// group/provider never leaks to another (no account-wide union), and a policy
-	// that carries no guardrail is genuinely unrestricted rather than being caught
-	// by some other policy's allowlist.
-	// Only consult guardrails when at least one candidate policy references one;
-	// a policy with no guardrail is unrestricted, so when none of them carries a
-	// guardrail the gate is a no-op and we skip the store read entirely.
 	if len(candidates) > 0 && anyPolicyHasGuardrails(candidates) {
 		guardrailsByID, gErr := m.loadGuardrailsByID(ctx, in.AccountID)
 		if gErr != nil {
@@ -323,13 +313,10 @@ func filterModelPermittedPolicies(policies []*types.Policy, byID map[string]*typ
 	return out
 }
 
-// policyPermitsModel reports whether a policy permits the requested model under
+// policyPermitsModel reports whether a policy permits the model. No
-// its attached guardrails. A policy with no allowlist-enabled guardrail is
+// allowlist-enabled guardrail = unrestricted (permits any, incl. empty);
-// unrestricted (permits any model, including an empty/undetermined one). A
+// otherwise the model must be in the union of its allowlists, so an
-// policy with one or more allowlist-enabled guardrails permits the model only
+// empty/undetermined model fails closed.
-// when it appears in the union of those allowlists; an empty/undetermined model
-// never matches a non-empty allowlist, so such a policy fails closed — the same
-// contract the proxy guardrail enforces for path-routed providers.
 func policyPermitsModel(p *types.Policy, byID map[string]*types.Guardrail, model string) bool {
 	if p == nil {
 		return false

management/internals/modules/agentnetwork/policyselect_model_test.go

@@ -120,10 +120,9 @@ func TestSelectPolicy_CaseInsensitiveModelMatch(t *testing.T) {
 	assert.True(t, res.Allow, "case/whitespace variants must match the allowlist entry")
 }
 
-// TestSelectPolicy_UnguardedPolicyIsUnrestricted is the false-deny fix: when
+// TestSelectPolicy_UnguardedPolicyIsUnrestricted is the false-deny fix: when two
-// two policies authorise the same (provider, group) and one carries no
+// policies authorise the same (provider, group) and one has no guardrail, that
-// guardrail, that un-guardrailed policy makes the request unrestricted — it is
+// policy makes the request unrestricted — not caught by the other's allowlist.
-// NOT caught by the other policy's allowlist.
 func TestSelectPolicy_UnguardedPolicyIsUnrestricted(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	mgr, mockStore := newSelectorMgr(t, ctrl)
@@ -146,10 +145,8 @@ func TestSelectPolicy_UnguardedPolicyIsUnrestricted(t *testing.T) {
 }
 
 // TestSelectPolicy_AllowlistDoesNotLeakAcrossGroups is the false-allow fix: a
-// model allowlisted only for grp-b must not be usable by a caller in grp-a,
+// model allowlisted only for grp-b must not be usable by a grp-a caller. The
-// even though both groups' policies target the same provider. The selector only
+// selector considers only policies applicable to the caller's groups.
-// considers policies applicable to the caller's groups, so grp-b's allowlist
-// never enters grp-a's decision.
 func TestSelectPolicy_AllowlistDoesNotLeakAcrossGroups(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	mgr, mockStore := newSelectorMgr(t, ctrl)

management/internals/modules/agentnetwork/synthesizer.go

@@ -235,12 +235,10 @@ func SynthesizeServices(ctx context.Context, s store.Store, accountID string) ([
 
 	mergedGuardrails := mergeGuardrails(enabledPolicies, guardrailsByID)
 	applyAccountCollectionControls(&mergedGuardrails, settings)
-	// The proxy guardrail is the fail-closed backstop, scoped per provider so it
+	// The proxy guardrail is a per-provider fail-closed backstop; the
-	// never applies one policy's allowlist to another provider's traffic. The
+	// authoritative per-policy/group decision is management's
-	// authoritative per-policy/group model decision lives in management's
+	// SelectPolicyForRequest. A provider lands in this map only when every
-	// SelectPolicyForRequest; a provider only lands in this map when EVERY policy
+	// authorising policy restricts models.
-	// authorising it restricts models, so an un-guardrailed policy leaves its
-	// providers unrestricted here.
 	providerAllowlists := buildProviderAllowlists(enabledPolicies, guardrailsByID)
 	guardrailJSON, err := marshalGuardrailConfig(providerAllowlists, mergedGuardrails.PromptCapture)
 	if err != nil {
@@ -852,15 +850,9 @@ func marshalGuardrailConfig(providerAllowlists map[string][]string, capture Merg
 	return out, nil
 }
 
-// buildProviderAllowlists computes the per-provider model allowlist the proxy
+// buildProviderAllowlists returns the proxy's per-provider backstop: a provider
-// guardrail enforces as a fail-closed backstop. A provider is included ONLY when
+// is included only when every authorising policy restricts models (their union);
-// every enabled policy that authorises it restricts models (has at least one
+// if any leaves it unrestricted it is omitted, so management decides per group.
-// allowlist-enabled guardrail); its list is then the union of those policies'
-// allowed models. If any authorising policy leaves models unrestricted, the
-// provider is omitted entirely — the proxy treats an absent provider as
-// unrestricted and defers to management's per-policy/group decision, so a
-// mixed set of policies can neither leak a model across providers nor wrongly
-// block an un-guardrailed policy's traffic.
 func buildProviderAllowlists(policies []*types.Policy, byID map[string]*types.Guardrail) map[string][]string {
 	type providerAcc struct {
 		models          map[string]struct{}
@@ -905,10 +897,9 @@ func buildProviderAllowlists(policies []*types.Policy, byID map[string]*types.Gu
 	return out
 }
 
-// policyModelAllowlist reports whether a policy restricts models (has at least
+// policyModelAllowlist reports whether a policy restricts models (has an
-// one allowlist-enabled guardrail) and the union of allowed models across those
+// allowlist-enabled guardrail) and the union of allowed models. Models are
-// guardrails. Models are returned verbatim; the proxy factory lowercases and
+// verbatim; the proxy factory lowercases/trims them at decode time.
-// trims them at decode time, matching the runtime compare.
 func policyModelAllowlist(p *types.Policy, byID map[string]*types.Guardrail) (bool, []string) {
 	restricted := false
 	var models []string
@@ -1070,11 +1061,9 @@ func unionSourceGroups(policies []*types.Policy) []string {
 	return out
 }
 
-// MergedGuardrails is the intermediate the synthesiser folds policy guardrails
+// MergedGuardrails is the synthesiser's fold target. Only prompt capture is
-// into. Only prompt capture is merged here — the model allowlist is emitted
+// merged here — the model allowlist is emitted per-provider, and
-// per-provider via buildProviderAllowlists, and token/budget/retention moved off
+// token/budget/retention moved onto Policy.Limits and account Settings.
-// guardrails onto Policy.Limits and account Settings — so this carries just the
-// prompt-capture decision the capture parsers and guardrail config consume.
 type MergedGuardrails struct {
 	PromptCapture MergedPromptCapture
 }
@@ -1084,17 +1073,10 @@ type MergedPromptCapture struct {
 	RedactPii bool `json:"redact_pii"`
 }
 
-// mergeGuardrails computes the prompt-capture portion of the effective
+// mergeGuardrails folds the referencing policies' guardrails into the
-// guardrail spec, given the referencing policies and the account's guardrail
+// prompt-capture decision only. The model allowlist is enforced per-policy/group
-// catalogue. Policy enabled-ness is the caller's responsibility — only enabled
+// in management and shipped per-provider; token/budget/retention live off
-// policies should be passed in.
+// guardrails now.
-//
-// The model allowlist is NOT merged here: it is enforced per-policy/group in
-// management (SelectPolicyForRequest) and shipped to the proxy per-provider via
-// buildProviderAllowlists, so a single account-wide union would be both
-// incorrect (leaks a model across groups/providers) and redundant. Token,
-// budget, and retention likewise live off guardrails now (Policy.Limits and
-// account Settings), so prompt capture is all that remains to merge.
 //
 // Merge rule — prompt capture: enabled if any policy enables it; redact_pii
 // sticks if any enabling policy turns it on.

management/internals/modules/agentnetwork/synthesizer_test.go

@@ -1031,12 +1031,10 @@ func TestSynthesizeServices_GuardrailMerge_AllowlistUnion_LimitsRestrictive(t *t
 
 	var cfg guardrailConfig
 	require.NoError(t, json.Unmarshal(guardrailJSON, &cfg), "guardrail config must unmarshal cleanly")
-	// Both policies restrict the same provider, so the proxy-side per-provider
+	// Both policies restrict the same provider, so the per-provider backstop
-	// backstop carries the union of their models. This coarse union is
+	// carries the union of their models — a coarse gate that management's
-	// deliberate: it can still let grp-a reach grp-b's model at the proxy layer,
+	// per-policy/group check narrows; it only blocks models outside the union
-	// which management's per-policy/group check (SelectPolicyForRequest) is the
+	// when management is down.
-	// one that rejects. The backstop only guarantees nothing outside the
-	// provider's union slips through when management is unreachable.
 	assert.ElementsMatch(t, []string{"gpt-5.4-mini", "gpt-5.4-pro"}, cfg.ProviderAllowlists["prov-1"],
 		"per-provider allowlist union must keep both models")
 }

proxy/internal/middleware/builtin/llm_guardrail/factory.go

@@ -13,17 +13,10 @@ import (
 // runtime path consumes the normalised allowlists; raw config is not
 // retained beyond construction.
 type Config struct {
-	// ProviderAllowlists maps a resolved provider id (the value llm_router
+	// ProviderAllowlists maps a resolved provider id (KeyLLMResolvedProviderID) to
-	// stamps as KeyLLMResolvedProviderID) to that provider's model allowlist. A
+	// its model allowlist. A provider present is restricted to those models; one
-	// provider present here is restricted to the listed models; a provider
+	// absent is unrestricted. Kept per-provider so one provider's list can't leak
-	// absent is unrestricted. The synthesiser only lists a provider when EVERY
+	// onto another.
-	// policy authorising it enables an allowlist, so a provider reachable by any
-	// un-guardrailed policy is intentionally absent (unrestricted) here and the
-	// precise per-policy/group decision is left to management. Keeping the gate
-	// per-provider — rather than one account-wide union — is what stops a model
-	// allowlisted for one provider from leaking onto another and stops an
-	// un-guardrailed policy's traffic from being blocked by an unrelated
-	// policy's allowlist.
 	ProviderAllowlists map[string][]string `json:"provider_allowlists,omitempty"`
 	PromptCapture      PromptCapture       `json:"prompt_capture"`
 }
@@ -65,11 +58,10 @@ func isEmptyJSON(raw []byte) bool {
 	return false
 }
 
-// normaliseConfig lowercases and trims allowlist entries so the runtime
+// normaliseConfig lowercases and trims allowlist entries for case-insensitive
-// match is case-insensitive. Empty entries are dropped. A provider whose
+// matching; empty entries drop. A provider whose entries all drop keeps an empty
-// entries all drop out keeps an empty (non-nil) list — an allowlist that
+// (non-nil) list — "deny every model" — distinct from an absent provider
-// permits nothing — which is the intended "deny every model" for that
+// (unrestricted).
-// provider, distinct from the provider being absent (unrestricted).
 func normaliseConfig(cfg Config) Config {
 	if len(cfg.ProviderAllowlists) == 0 {
 		cfg.ProviderAllowlists = nil

proxy/internal/middleware/builtin/llm_guardrail/middleware.go

@@ -111,20 +111,16 @@ func (m *Middleware) Invoke(_ context.Context, in *middleware.Input) (*middlewar
 // is a no-op.
 func (m *Middleware) Close() error { return nil }
 
-// evaluateAllowlist returns a deny Output when the allowlist for the request's
+// evaluateAllowlist denies when the resolved provider's allowlist rejects the
-// resolved provider rejects the model. A nil return means the request should
+// model; nil means proceed. Scoped to the provider llm_router resolved, so an
-// proceed. The allowlist is scoped to the provider llm_router resolved: a
+// unrestricted provider (absent from config) is never caught by another's list.
-// provider absent from the config is unrestricted, so an un-guardrailed policy's
-// traffic is never caught by an unrelated provider's allowlist.
 func (m *Middleware) evaluateAllowlist(providerID, model string, modelPresent bool) *middleware.Output {
 	if len(m.cfg.ProviderAllowlists) == 0 {
 		return nil
 	}
-	// Restrictions exist for this account but the resolved provider is unknown,
+	// Restrictions exist but the resolved provider is unknown, so we can't tell
-	// so we cannot tell whether this request is bound for a restricted provider.
+	// if this request targets a restricted provider — fail closed. llm_router
-	// Fail closed rather than wave it through. In practice llm_router always
+	// normally stamps the provider first, so this is a defensive guard.
-	// stamps the resolved provider before the guardrail runs (or denies first),
-	// so this is a defensive guard, not the common path.
 	if providerID == "" {
 		return denyModel("", denyCodeModelUnknown, denyMessageModelUnknown, denyReasonModelUnknown)
 	}
@@ -135,10 +131,8 @@ func (m *Middleware) evaluateAllowlist(providerID, model string, modelPresent bo
 		return nil
 	}
 	// Fail closed: with an allowlist in effect for this provider, a request whose
-	// model the upstream parser could not extract (absent or empty) must be
+	// model the parser couldn't extract (absent/empty) is denied. This enforces
-	// denied rather than allowed. This is what enforces the allowlist for
+	// the allowlist for path-routed providers (Bedrock, Vertex) with no body model.
-	// URL/path-routed providers (Bedrock, Vertex, ...) whose model lives outside
-	// the JSON body.
 	if !modelPresent || normaliseModel(model) == "" {
 		return denyModel("", denyCodeModelUnknown, denyMessageModelUnknown, denyReasonModelUnknown)
 	}

proxy/internal/middleware/builtin/llm_guardrail/middleware_test.go

@@ -164,10 +164,9 @@ func TestAllowlistEmptyListAllowsMissingModel(t *testing.T) {
 }
 
 func TestUnrestrictedProviderAllowsAnyModel(t *testing.T) {
-	// testProvider is restricted, but the request resolved to otherProvider,
+	// The request resolved to otherProvider, which has no allowlist, so its
-	// which carries no allowlist. Its traffic must not be caught by the other
+	// traffic must not be caught by testProvider's restriction — the
-	// provider's restriction — this is the cross-provider-leak / false-deny
+	// cross-provider-leak / false-deny guard.
-	// guard. Management owns any per-policy/group decision for otherProvider.
 	mw := New(providerCfg("gpt-4o"))
 	out, err := mw.Invoke(context.Background(), newInputProvider(otherProvider,
 		middleware.KV{Key: middleware.KeyLLMModel, Value: "claude-opus-4"},