Merge branch 'main' into agent-network-validate-proxy-cluster

e2e/agentnetwork/agent_config_test.go

@@ -0,0 +1,143 @@
+//go:build e2e
+
+package agentnetwork
+
+import (
+	"context"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/shared/management/http/api"
+)
+
+// joinGroup places the PAT's own user into the group so caller-scoped answers
+// (GET /api/agent-network/agent-config) see the policies sourced from it, and
+// restores the previous auto-groups on cleanup. Self-service updates of one's
+// own auto_groups are permitted for every role, so this needs no second user.
+func joinGroup(t *testing.T, ctx context.Context, groupID string) {
+	t.Helper()
+	me, err := srv.API().Users.Current(ctx)
+	require.NoError(t, err, "read current user")
+	before := append([]string(nil), me.AutoGroups...)
+	_, err = srv.API().Users.Update(ctx, me.Id, api.PutApiUsersUserIdJSONRequestBody{
+		Role:       me.Role,
+		IsBlocked:  me.IsBlocked,
+		AutoGroups: append(append([]string(nil), before...), groupID),
+	})
+	require.NoError(t, err, "add the caller to the policy source group")
+	t.Cleanup(func() {
+		_, _ = srv.API().Users.Update(context.Background(), me.Id, api.PutApiUsersUserIdJSONRequestBody{
+			Role:       me.Role,
+			IsBlocked:  me.IsBlocked,
+			AutoGroups: before,
+		})
+	})
+}
+
+// configProvider returns the agent-config entry for the named provider, nil
+// when the answer does not offer it. The suite shares one account, so other
+// tests' fixtures may add unrelated providers to the caller's answer.
+func configProvider(cfg api.AgentNetworkAgentConfig, name string) *api.AgentNetworkAgentConfigProvider {
+	for i := range cfg.Providers {
+		if cfg.Providers[i].Name == name {
+			return &cfg.Providers[i]
+		}
+	}
+	return nil
+}
+
+// TestAgentConfigAllowlistOfDeclaredModels reproduces the post-#7221 field
+// report: a provider carrying a declared model set plus a policy guardrail
+// whose allowlist holds those same declared ids must advertise the models on
+// GET /api/agent-network/agent-config — the guardrail was built FROM the
+// provider's model list (the dashboard's allowlist picker persists the
+// declared ids verbatim), so nothing about the setup excludes them.
+//
+// The plain case passes today. The path-style case (Bedrock; Vertex has the
+// same shape) fails: the declared id is compared through the proxy parser's
+// canonical form (region prefix and version suffix stripped) while the
+// allowlist entry is not, so the raw-vs-raw pair never intersects and the
+// caller sees an empty model list. The same one-sided normalization sits in
+// policyPermitsModel, so the proxy also denies the model at request time —
+// the guardrail meant to allow exactly this model turns it off end to end.
+func TestAgentConfigAllowlistOfDeclaredModels(t *testing.T) {
+	ctx := context.Background()
+
+	cases := []struct {
+		name      string
+		catalogID string
+		upstream  string
+		declared  string
+	}{
+		{
+			name:      "plain-declared-id",
+			catalogID: "openai_api",
+			upstream:  "https://api.openai.com",
+			declared:  "gpt-4o-mini",
+		},
+		{
+			// The operator declares the id AWS issues — region-prefixed
+			// inference profile with a version suffix — and the allowlist
+			// picker copies it as-is.
+			name:      "bedrock-declared-id",
+			catalogID: "bedrock_api",
+			upstream:  "https://bedrock-runtime.eu-central-1.amazonaws.com",
+			declared:  "eu.anthropic.claude-sonnet-4-5-20250929-v1:0",
+		},
+	}
+
+	for _, tc := range cases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			grp, err := srv.API().Groups.Create(ctx, api.PostApiGroupsJSONRequestBody{Name: "e2e-agentcfg-" + tc.name})
+			require.NoError(t, err, "create source group")
+			t.Cleanup(func() { _ = srv.API().Groups.Delete(context.Background(), grp.Id) })
+
+			joinGroup(t, ctx, grp.Id)
+
+			providerName := "e2e-agentcfg-" + tc.name
+			prov, err := srv.CreateProvider(ctx, api.AgentNetworkProviderRequest{
+				Name:        providerName,
+				ProviderId:  tc.catalogID,
+				UpstreamUrl: tc.upstream,
+				ApiKey:      ptr("sk-dummy-e2e-key"),
+				Enabled:     ptr(true),
+				Models:      &[]api.AgentNetworkProviderModel{{Id: tc.declared, InputPer1k: 0.001, OutputPer1k: 0.002}},
+			})
+			require.NoError(t, err, "create provider")
+			t.Cleanup(func() { _ = srv.DeleteProvider(context.Background(), prov.Id) })
+
+			// Allowlist exactly the declared model, the way the dashboard
+			// builds a guardrail from the provider's model list.
+			var gr api.AgentNetworkGuardrailRequest
+			gr.Name = "e2e-agentcfg-" + tc.name
+			gr.Checks.ModelAllowlist.Enabled = true
+			gr.Checks.ModelAllowlist.Models = []string{tc.declared}
+			guard, err := srv.CreateGuardrail(ctx, gr)
+			require.NoError(t, err, "create guardrail")
+			t.Cleanup(func() { _ = srv.DeleteGuardrail(context.Background(), guard.Id) })
+
+			pol, err := srv.CreatePolicy(ctx, api.AgentNetworkPolicyRequest{
+				Name:                   "e2e-agentcfg-" + tc.name,
+				Enabled:                ptr(true),
+				SourceGroups:           []string{grp.Id},
+				DestinationProviderIds: []string{prov.Id},
+				GuardrailIds:           &[]string{guard.Id},
+			})
+			require.NoError(t, err, "create policy")
+			t.Cleanup(func() { _ = srv.DeletePolicy(context.Background(), pol.Id) })
+
+			cfg, err := srv.GetAgentConfig(ctx)
+			require.NoError(t, err, "read the caller-scoped agent config")
+			require.True(t, cfg.Configured, "the account endpoint is bootstrapped by TestMain")
+
+			entry := configProvider(cfg, providerName)
+			require.NotNil(t, entry, "the policy authorizes the caller for the provider, so it must be offered")
+			assert.False(t, entry.AllModelsAllowed, "an allowlist guardrail restricts the provider")
+			assert.Equal(t, []string{tc.declared}, entry.Models,
+				"the allowlist holds the provider's own declared id, so that model must be advertised")
+		})
+	}
+}

e2e/agentnetwork/credential_check_live_test.go

@@ -0,0 +1,235 @@
+//go:build e2e
+
+package agentnetwork
+
+import (
+	"context"
+	"net/http"
+	"os"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/shared/management/client/rest"
+	"github.com/netbirdio/netbird/shared/management/http/api"
+)
+
+// credentialCase is one vendor to try the save-time check against. The key is
+// the real one the suite already sources; corrupting it is what produces the
+// refusal, so the pair of cases differ only in the credential.
+type credentialCase struct {
+	name      string
+	catalogID string
+	upstream  string
+	apiKey    string
+}
+
+// liveCredentialCases mirrors the discovery matrix's env gating so a partial
+// key set still yields partial coverage. Vertex is left out: its credential is
+// a service-account keyfile, and mangling one produces a client-side parse
+// failure rather than the vendor refusal this is about.
+func liveCredentialCases() []credentialCase {
+	var cases []credentialCase
+
+	if k := os.Getenv("OPENAI_TOKEN"); k != "" {
+		cases = append(cases, credentialCase{
+			name: "openai", catalogID: "openai_api",
+			upstream: "https://api.openai.com", apiKey: k,
+		})
+	}
+	if k := os.Getenv("ANTHROPIC_TOKEN"); k != "" {
+		cases = append(cases, credentialCase{
+			name: "anthropic", catalogID: "anthropic_api",
+			upstream: "https://api.anthropic.com", apiKey: k,
+		})
+	}
+	if k := os.Getenv("AWS_BEARER_TOKEN_BEDROCK"); k != "" {
+		region := os.Getenv("AWS_REGION")
+		if region == "" {
+			region = "eu-central-1"
+		}
+		cases = append(cases, credentialCase{
+			name: "bedrock", catalogID: "bedrock_api",
+			upstream: "https://bedrock-runtime." + region + ".amazonaws.com", apiKey: k,
+		})
+	}
+
+	return cases
+}
+
+// TestLiveProviderCredentialCheck drives the save-time check against the real
+// vendors. A unit test can only assert that a mocked refusal is classified;
+// what it cannot show is that these vendors refuse a bad key on their listing
+// endpoint at all, which is the assumption the whole feature rests on.
+//
+// The good-key case matters just as much as the bad one: a check that refused
+// everything would pass a test asserting only the refusal, and would make the
+// product unusable.
+//
+// The suite asserts on the vendors themselves, so it inherits their
+// availability: the check blocks on 5xx and 429 by design, and a vendor outage
+// or a rate limit during a run fails "a good credential saves" with a
+// perfectly valid key. There is no retry here on purpose — a retry loop would
+// also mask the outage classification these tests exist to prove. Re-run the
+// job.
+func TestLiveProviderCredentialCheck(t *testing.T) {
+	cases := liveCredentialCases()
+	if len(cases) == 0 {
+		t.Skip("no live provider credentials in the environment; source ~/.llm-keys to run")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
+	defer cancel()
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Run("a good credential saves", func(t *testing.T) {
+				prov, err := srv.CreateProvider(ctx, credentialProviderRequest(tc, "e2e-cred-ok-"+tc.name, tc.apiKey))
+				require.NoError(t, err, "the suite's own credential must pass its check")
+				t.Cleanup(func() { _ = srv.DeleteProvider(context.Background(), prov.Id) })
+				require.NotEmpty(t, prov.Id)
+			})
+
+			t.Run("a rejected credential is refused", func(t *testing.T) {
+				_, err := srv.CreateProvider(ctx, credentialProviderRequest(tc, "e2e-cred-bad-"+tc.name, corrupt(tc.apiKey)))
+				require.Error(t, err, "a key the vendor rejects must not save")
+
+				var apiErr *rest.APIError
+				require.ErrorAs(t, err, &apiErr)
+				require.Equal(t, http.StatusUnprocessableEntity, apiErr.StatusCode,
+					"a refused credential is the caller's problem to fix, not a server fault")
+				require.Contains(t, strings.ToLower(apiErr.Message), "rejected the credential",
+					"the message must name the credential rather than the url")
+
+				// The record must be absent, not merely unusable: a provider
+				// saved despite its check is the state this prevents.
+				all, listErr := srv.ListProviders(ctx)
+				require.NoError(t, listErr)
+				for _, p := range all {
+					require.NotEqual(t, "e2e-cred-bad-"+tc.name, p.Name, "a refused provider must not be stored")
+				}
+			})
+		})
+	}
+}
+
+// TestLiveProviderUrlCheck points a real credential at a host that is not the
+// vendor's API. It is the half of the split a wrong key cannot exercise: the
+// operator has to be told the URL is at fault while their key is fine.
+//
+// One vendor, deliberately. The transport classification under test happens
+// before any vendor is reached, so running it per configured vendor would
+// repeat the same code path and multiply the wall-clock of a suite that
+// already creates real records. cases[0] is whichever vendor the environment
+// supplies first.
+func TestLiveProviderUrlCheck(t *testing.T) {
+	cases := liveCredentialCases()
+	if len(cases) == 0 {
+		t.Skip("no live provider credentials in the environment; source ~/.llm-keys to run")
+	}
+	tc := cases[0]
+
+	ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
+	defer cancel()
+
+	// A name that resolves nowhere. The check has to reach a verdict without
+	// the vendor's help, which is the transport half of the classification.
+	req := credentialProviderRequest(tc, "e2e-cred-badurl", tc.apiKey)
+	req.UpstreamUrl = "https://not-a-real-vendor-host.netbird-e2e.invalid"
+
+	_, err := srv.CreateProvider(ctx, req)
+	require.Error(t, err, "an upstream that does not resolve must not save")
+
+	var apiErr *rest.APIError
+	require.ErrorAs(t, err, &apiErr)
+	require.Equal(t, http.StatusUnprocessableEntity, apiErr.StatusCode)
+	require.Contains(t, strings.ToLower(apiErr.Message), "could not be reached",
+		"the message must name the url rather than the credential")
+
+	// An error is not the same fact as an absent record: a handler that saved
+	// first and reported afterwards would satisfy everything above.
+	all, listErr := srv.ListProviders(ctx)
+	require.NoError(t, listErr)
+	for _, p := range all {
+		require.NotEqual(t, "e2e-cred-badurl", p.Name, "a refused provider must not be stored")
+	}
+}
+
+// TestLiveProviderUpdateKeepsTheWorkingKey is the state the check exists to
+// prevent on the update path: a rejected rotation that has already replaced
+// the credential would take a working provider down.
+//
+// Also one vendor: the behaviour is in the manager's merge, not in any
+// vendor's response, and each run creates and mutates a real provider record.
+func TestLiveProviderUpdateKeepsTheWorkingKey(t *testing.T) {
+	cases := liveCredentialCases()
+	if len(cases) == 0 {
+		t.Skip("no live provider credentials in the environment; source ~/.llm-keys to run")
+	}
+	tc := cases[0]
+
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
+	defer cancel()
+
+	prov, err := srv.CreateProvider(ctx, credentialProviderRequest(tc, "e2e-cred-rotate", tc.apiKey))
+	require.NoError(t, err)
+	t.Cleanup(func() { _ = srv.DeleteProvider(context.Background(), prov.Id) })
+
+	rotation := credentialProviderRequest(tc, "e2e-cred-rotate", corrupt(tc.apiKey))
+	_, err = srv.UpdateProvider(ctx, prov.Id, rotation)
+	require.Error(t, err, "a rotation the vendor rejects must not be stored")
+
+	var apiErr *rest.APIError
+	require.ErrorAs(t, err, &apiErr)
+	require.Equal(t, http.StatusUnprocessableEntity, apiErr.StatusCode)
+
+	// The stored key is never returned by the API, so the proof that it
+	// survived is that an edit which reuses it still passes its check. A
+	// replaced key would fail here exactly as the rotation just did.
+	//
+	// The trailing slash is what makes that an actual check: an edit touching
+	// neither the url, the key nor the catalog entry is stored without asking
+	// the vendor anything, so a rename alone would pass whatever is on the
+	// record. Only the host is read out of the upstream, so the same vendor is
+	// reached — but the string differs, and the check runs.
+	recheck := credentialProviderRequest(tc, "e2e-cred-rotate-renamed", "")
+	recheck.UpstreamUrl = tc.upstream + "/"
+	updated, err := srv.UpdateProvider(ctx, prov.Id, recheck)
+	require.NoError(t, err, "the working key must still be the stored one")
+	require.Equal(t, "e2e-cred-rotate-renamed", updated.Name)
+}
+
+// credentialProviderRequest builds a create/update body for a case. An empty apiKey is
+// omitted rather than sent blank, which is how the form asks to keep whatever
+// is already stored.
+func credentialProviderRequest(tc credentialCase, name, apiKey string) api.AgentNetworkProviderRequest {
+	req := api.AgentNetworkProviderRequest{
+		Name:        name,
+		ProviderId:  tc.catalogID,
+		UpstreamUrl: tc.upstream,
+		Enabled:     ptr(true),
+	}
+	if apiKey != "" {
+		req.ApiKey = &apiKey
+	}
+	return req
+}
+
+// corrupt returns a key the vendor will reject while keeping the shape of the
+// original. Replacing the last character rather than appending keeps any
+// length or prefix validation satisfied, so the refusal comes from the vendor
+// checking the secret rather than from it rejecting an obviously malformed
+// one.
+func corrupt(key string) string {
+	if key == "" {
+		return key
+	}
+	last := key[len(key)-1]
+	replacement := byte('A')
+	if last == 'A' {
+		replacement = 'B'
+	}
+	return key[:len(key)-1] + string(replacement)
+}

e2e/agentnetwork/guardrail_declared_ids_test.go

@@ -0,0 +1,132 @@
+//go:build e2e
+
+package agentnetwork
+
+import (
+	"context"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/e2e/harness"
+	"github.com/netbirdio/netbird/shared/management/http/api"
+)
+
+// TestModelAllowlistOfDeclaredIDsServed drives the setup an operator actually
+// builds for a path-routed provider: the models are declared in the form the
+// vendor issues (Bedrock's region-prefixed, versioned inference-profile id;
+// Vertex's model@version), and the guardrail allowlist is built from that
+// declared list — the dashboard's allowlist picker persists the declared ids
+// verbatim. A request for the declared model must be served end to end, and a
+// model outside the allowlist must still be denied.
+//
+// TestModelAllowlistEnforced never caught this because it registers and
+// allowlists the pre-normalized catalog form (see the catalogModel comment
+// there and the one in providerRequest: "register the normalized form here or
+// routing fails as model_not_routable") — the harness encoded the
+// canonicalization workaround instead of the shape operators configure.
+func TestModelAllowlistOfDeclaredIDsServed(t *testing.T) {
+	var providers []providerCase
+	for _, pc := range availableProviders() {
+		if pc.kind == harness.WireBedrock || pc.kind == harness.WireVertex {
+			providers = append(providers, pc)
+		}
+	}
+	if len(providers) == 0 {
+		t.Skip("no path-routed provider keys set (AWS_BEARER_TOKEN_BEDROCK / GOOGLE_VERTEX_*); source ~/.llm-keys")
+	}
+
+	ctx, cancel := context.WithTimeout(context.Background(), 20*time.Minute)
+	defer cancel()
+
+	grp, err := srv.API().Groups.Create(ctx, api.PostApiGroupsJSONRequestBody{Name: "e2e-declared-allowlist"})
+	require.NoError(t, err, "create group")
+	t.Cleanup(func() { _ = srv.API().Groups.Delete(context.Background(), grp.Id) })
+
+	ephemeral := false
+	sk, err := srv.API().SetupKeys.Create(ctx, api.PostApiSetupKeysJSONRequestBody{
+		Name:       "e2e-declared-allowlist-client",
+		Type:       "reusable",
+		ExpiresIn:  86400,
+		UsageLimit: 0,
+		AutoGroups: []string{grp.Id},
+		Ephemeral:  &ephemeral,
+	})
+	require.NoError(t, err, "mint setup key")
+	t.Cleanup(func() { _ = srv.API().SetupKeys.Delete(context.Background(), sk.Id) })
+
+	// Providers declaring the raw vendor-issued model id — NOT the normalized
+	// catalog form providerRequest would register.
+	ids := make([]string, 0, len(providers))
+	declared := make([]string, 0, len(providers))
+	for _, pc := range providers {
+		req := providerRequest(pc)
+		req.Models = &[]api.AgentNetworkProviderModel{{Id: pc.model, InputPer1k: 0.001, OutputPer1k: 0.002}}
+		prov, perr := srv.CreateProvider(ctx, req)
+		require.NoError(t, perr, "create provider %s", pc.name)
+		id := prov.Id
+		ids = append(ids, id)
+		declared = append(declared, pc.model)
+		t.Cleanup(func() { _ = srv.DeleteProvider(context.Background(), id) })
+	}
+
+	// Guardrail allowlisting the declared ids verbatim, the way the dashboard
+	// builds an allowlist from the providers' model lists.
+	var gr api.AgentNetworkGuardrailRequest
+	gr.Name = "e2e-declared-allowlist"
+	gr.Checks.ModelAllowlist.Enabled = true
+	gr.Checks.ModelAllowlist.Models = declared
+	guard, err := srv.CreateGuardrail(ctx, gr)
+	require.NoError(t, err, "create guardrail")
+	t.Cleanup(func() { _ = srv.DeleteGuardrail(context.Background(), guard.Id) })
+
+	enabled := true
+	pol, err := srv.CreatePolicy(ctx, api.AgentNetworkPolicyRequest{
+		Name:                   "e2e-declared-allowlist",
+		Enabled:                &enabled,
+		SourceGroups:           []string{grp.Id},
+		DestinationProviderIds: ids,
+		GuardrailIds:           &[]string{guard.Id},
+	})
+	require.NoError(t, err, "create policy")
+	t.Cleanup(func() { _ = srv.DeletePolicy(context.Background(), pol.Id) })
+
+	settings, err := srv.GetSettings(ctx)
+	require.NoError(t, err, "read settings for endpoint")
+	require.NotEmpty(t, settings.Endpoint, "agent-network endpoint must be assigned")
+
+	proxyToken, err := srv.CreateProxyTokenCLI(ctx, "e2e-proxy-declared-allowlist")
+	require.NoError(t, err, "mint proxy token via CLI")
+	px, err := harness.StartProxy(ctx, srv, proxyToken)
+	require.NoError(t, err, "start proxy")
+	t.Cleanup(func() { _ = px.Terminate(context.Background()) })
+
+	cl, err := harness.StartClient(ctx, srv, sk.Key)
+	require.NoError(t, err, "start client")
+	t.Cleanup(func() { _ = cl.Terminate(context.Background()) })
+
+	require.NoError(t, cl.WaitConnected(ctx, 90*time.Second), "client must connect to management")
+	// Probe first: the GET resolves the endpoint (DNS error fails) and its first packet wakes the lazy proxy peer, so WaitProxyPeer sees it connected; any HTTP status counts.
+	proxyIP, err := cl.ResolveProxyIP(ctx, settings.Endpoint)
+	require.NoError(t, err, "resolve agent-network endpoint to proxy IP")
+	if err := cl.WaitProxyPeer(ctx, 180*time.Second); err != nil {
+		t.Fatalf("client did not see the proxy peer: %v\n=== proxy logs ===\n%s", err, px.Logs(context.Background()))
+	}
+
+	for _, pc := range providers {
+		pc := pc
+		t.Run(pc.name, func(t *testing.T) {
+			// The model the operator declared and allowlisted is served end to
+			// end: the route must claim it and the guardrail must permit it,
+			// both through the canonicalization the parser applies at request
+			// time — whatever id form the operator configured.
+			assert.Equal(t, 200, sendModel(ctx, t, cl, settings.Endpoint, proxyIP, pc, pc.model),
+				"the declared and allowlisted model must be served for %s", pc.name)
+			// A model outside the allowlist stays denied.
+			assert.Equal(t, 403, sendModel(ctx, t, cl, settings.Endpoint, proxyIP, pc, disallowedModel(pc)),
+				"model outside the allowlist must be denied for %s", pc.name)
+		})
+	}
+}

e2e/agentnetwork/management_test.go

@@ -16,14 +16,20 @@ import (
 
 func ptr[T any](v T) *T { return &v }
 
-// newProvider creates an OpenAI-catalog provider with a dummy key (these tests
-// never call the upstream) and registers cleanup.
+// newProvider creates an OpenAI-catalog provider these tests can hang a policy
+// off, and registers cleanup. Nothing here calls the upstream.
 func newProvider(t *testing.T, ctx context.Context, name string) api.AgentNetworkProvider {
 	t.Helper()
+	// A provider save is credential-checked against the vendor, and every
+	// caller here wants a provider row to hang a policy off rather than a
+	// working upstream. A private address is left unchecked — the proxy would
+	// reach it through the tunnel, management cannot reach it at all — which
+	// keeps this fixture independent of whether the run has vendor keys, and
+	// covers the unchecked-provider-still-saves path while it is at it.
 	prov, err := srv.CreateProvider(ctx, api.AgentNetworkProviderRequest{
 		Name:        name,
 		ProviderId:  "openai_api",
-		UpstreamUrl: "https://api.openai.com",
+		UpstreamUrl: "https://10.255.255.1",
 		ApiKey:      ptr("sk-dummy-e2e-key"),
 	})
 	require.NoError(t, err, "create provider %q", name)