[management] Check a provider's url and credential before saving it

The provider form accepted anything and found out later. A typo in the
upstream, a key pasted a character short, an AWS access key in a field that
wants a Bedrock API key — all saved cleanly, then surfaced minutes later as a
failed request or an empty model picker, with nothing pointing back at the
record that caused it.

CreateProvider now spends the credential once against the vendor's own model
listing, and UpdateProvider does the same when the upstream or the key
changed — only then, so renames, model rows and price edits neither wait on a
vendor nor fail because one is having a bad day. Both run before the store
write, so a rejected rotation leaves the working key exactly where it was.

The check reuses the discovery Fetch rather than a lighter status probe. It
exercises the path the model picker will take, so a URL answering 200 with a
login page fails here instead of passing a status check and producing an empty
picker later.

Failures that mean "we cannot ask" are not failures: a gateway with no listing
endpoint, a Bedrock record behind a proxy where no control-plane host can be
derived, and a self-hosted endpoint on a private network that the proxy
reaches through the tunnel but management cannot. None of those are evidence
the record is wrong, and refusing them would make this a lockout.

Discovery failures are typed for it. The message an operator sees carries no
status code and never echoes their URL — WriteError lowercases it, and paths
are case-sensitive, so an echoed URL would come back altered and describe
something they did not type. The vendor's status is logged instead.

management/internals/modules/agentnetwork/credentialcheck.go

@@ -0,0 +1,120 @@
+package agentnetwork
+
+import (
+	"context"
+	"errors"
+	"net/http"
+
+	log "github.com/sirupsen/logrus"
+
+	"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/modeldiscovery"
+	"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+// The provider form used to accept anything and find out later. A typo in the
+// upstream URL, a key pasted with a character missing, an AWS access key in a
+// field that wants a Bedrock API key — all saved cleanly, and surfaced as a
+// failed request or an empty model picker some minutes later, with nothing
+// pointing back at the record that caused it.
+//
+// checkProviderCredential closes that gap by spending the credential once, at
+// save time, against the vendor's own model listing.
+
+// ModelLister is the vendor-facing half of the credential check.
+// modeldiscovery.Client is the only production implementation; it is an
+// interface because the check runs on a write path, so without a seam every
+// test that saves a provider would reach a vendor over the network to do it.
+type ModelLister interface {
+	Fetch(ctx context.Context, req modeldiscovery.Request) ([]modeldiscovery.Model, error)
+}
+
+// checkProviderCredential asks the vendor whether this record's upstream and
+// credential actually work, and refuses the write if they do not.
+//
+// Deliberately reuses the discovery Fetch rather than a lighter status probe:
+// it exercises the exact path the model picker will take, so a URL that
+// answers 200 with a login page fails here instead of passing a status check
+// and producing an empty picker later.
+//
+// A provider the check cannot cover is saved, not blocked. That covers the
+// eleven catalog entries with no listing endpoint, a Bedrock record whose
+// upstream is proxied so no control-plane host can be derived, and a
+// self-hosted endpoint on a private network that the proxy reaches through
+// the tunnel but management cannot reach at all. None of those are evidence
+// the record is wrong.
+func (m *managerImpl) checkProviderCredential(ctx context.Context, provider *types.Provider) error {
+	_, err := m.modelDiscovery.Fetch(ctx, modeldiscovery.Request{
+		CatalogID:   provider.ProviderID,
+		UpstreamURL: provider.UpstreamURL,
+		APIKey:      provider.APIKey,
+	})
+	if err == nil {
+		return nil
+	}
+
+	message, blocking := credentialCheckFailure(err)
+	if !blocking {
+		log.WithContext(ctx).Debugf("agent network provider %s not credential-checked: %v", provider.ProviderID, err)
+		return nil
+	}
+
+	// The operator's message carries no status code, so the number lives here
+	// or nowhere. WriteError logs whatever we return, which is the message
+	// alone, so a support question about a 403 has nothing to go on without
+	// this line.
+	log.WithContext(ctx).Infof("agent network provider %s failed its credential check: %v", provider.ProviderID, err)
+
+	return status.Errorf(status.InvalidArgument, "%s", message)
+}
+
+// credentialCheckFailure renders a discovery failure as the sentence the
+// provider form shows, and reports whether it should block the write.
+//
+// The strings are written to survive WriteError lowercasing them, and they
+// never echo the operator's URL: paths are case-sensitive, so an echoed URL
+// would come back altered and describe something they did not type.
+func credentialCheckFailure(err error) (message string, blocking bool) {
+	// Not checkable. The record may be perfectly good; we simply have no way
+	// to ask, so saying nothing is more honest than reporting a failure.
+	switch {
+	case errors.Is(err, modeldiscovery.ErrNoDiscovery),
+		errors.Is(err, modeldiscovery.ErrNoDiscoveryHost),
+		errors.Is(err, modeldiscovery.ErrPrivateHost):
+		return "", false
+	}
+
+	var vendor *modeldiscovery.VendorStatusError
+	if errors.As(err, &vendor) {
+		switch vendor.Status {
+		case http.StatusUnauthorized, http.StatusForbidden:
+			return "the provider rejected the credential", true
+		case http.StatusNotFound, http.StatusMethodNotAllowed:
+			return "the upstream url did not answer a model listing", true
+		default:
+			// Everything else the vendor chose to answer with, 5xx and 429
+			// included. A vendor outage blocks the write: working around it is
+			// not this check's job, and saving a record we could not verify
+			// would put the operator back where they started.
+			return "the provider returned an error", true
+		}
+	}
+
+	var unreachable *modeldiscovery.UnreachableError
+	if errors.As(err, &unreachable) {
+		if reason := unreachable.Reason(); reason != "" {
+			return "the upstream url could not be reached: " + reason, true
+		}
+		return "the upstream url could not be reached", true
+	}
+
+	if errors.Is(err, modeldiscovery.ErrUnparseableListing) {
+		return "the upstream url answered, but not with a model listing", true
+	}
+
+	// Anything left is ours, not theirs — a malformed request this code built,
+	// or a catalog entry that does not match its parser. Blocking is still
+	// right: we did not verify the record, and a save that silently skipped
+	// its check is the thing this feature exists to prevent.
+	return "the provider could not be checked", true
+}

management/internals/modules/agentnetwork/credentialcheck_test.go

@@ -0,0 +1,384 @@
+package agentnetwork
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"fmt"
+	"net"
+	"os"
+	"syscall"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/modeldiscovery"
+	"github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types"
+	"github.com/netbirdio/netbird/management/server/permissions/modules"
+	"github.com/netbirdio/netbird/management/server/permissions/operations"
+	"github.com/netbirdio/netbird/management/server/store"
+	"github.com/netbirdio/netbird/shared/management/status"
+)
+
+// stubLister stands in for the vendor on the write path. It records what it
+// was asked so a test can assert not only that the check ran, but that it ran
+// against the right upstream and the right credential — and, for an edit that
+// touches neither, that it did not run at all.
+type stubLister struct {
+	err      error
+	requests []modeldiscovery.Request
+}
+
+func (s *stubLister) Fetch(_ context.Context, req modeldiscovery.Request) ([]modeldiscovery.Model, error) {
+	s.requests = append(s.requests, req)
+	if s.err != nil {
+		return nil, s.err
+	}
+	return []modeldiscovery.Model{{ID: "a-model", PricingKnown: true}}, nil
+}
+
+func (s *stubLister) calls() int { return len(s.requests) }
+
+func (s *stubLister) only(t *testing.T) modeldiscovery.Request {
+	t.Helper()
+	require.Len(t, s.requests, 1, "the vendor must be asked exactly once")
+	return s.requests[0]
+}
+
+// TestCredentialCheckFailure_SeparatesTheUrlFromTheCredential is the contract
+// the provider form is written against: an operator gets told which of the two
+// fields they have to look at, and the message says so without a status code
+// and without echoing the URL back at them.
+func TestCredentialCheckFailure_SeparatesTheUrlFromTheCredential(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{
+			name: "401 is the credential",
+			err:  &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 401},
+			want: "the provider rejected the credential",
+		},
+		{
+			name: "403 is the credential",
+			err:  &modeldiscovery.VendorStatusError{Provider: "Bedrock", Status: 403},
+			want: "the provider rejected the credential",
+		},
+		{
+			// The host authenticated us fine and then said it has no such
+			// endpoint, which is the URL being wrong rather than the key.
+			name: "404 is the url",
+			err:  &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 404},
+			want: "the upstream url did not answer a model listing",
+		},
+		{
+			name: "405 is the url",
+			err:  &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 405},
+			want: "the upstream url did not answer a model listing",
+		},
+		{
+			name: "500 is the vendor",
+			err:  &modeldiscovery.VendorStatusError{Provider: "Anthropic", Status: 500},
+			want: "the provider returned an error",
+		},
+		{
+			name: "503 is the vendor",
+			err:  &modeldiscovery.VendorStatusError{Provider: "Anthropic", Status: 503},
+			want: "the provider returned an error",
+		},
+		{
+			name: "429 is the vendor",
+			err:  &modeldiscovery.VendorStatusError{Provider: "Anthropic", Status: 429},
+			want: "the provider returned an error",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			got, blocking := credentialCheckFailure(tc.err)
+			require.True(t, blocking, "a vendor refusal must block the write")
+			require.Equal(t, tc.want, got)
+		})
+	}
+}
+
+// TestCredentialCheckFailure_NamesTheTransportFault covers the failures that
+// never reached the vendor. The distinction inside them is worth keeping: a
+// refused connection is a wrong port and an unknown host is a wrong hostname,
+// and an operator staring at a URL they believe in needs to be told which.
+func TestCredentialCheckFailure_NamesTheTransportFault(t *testing.T) {
+	cases := []struct {
+		name string
+		err  error
+		want string
+	}{
+		{
+			name: "unknown host",
+			err:  &net.DNSError{Err: "no such host", Name: "api.exmaple.com", IsNotFound: true},
+			want: "the upstream url could not be reached: no such host",
+		},
+		{
+			name: "dns failure that is not a missing name",
+			err:  &net.DNSError{Err: "server misbehaving", Name: "api.example.com"},
+			want: "the upstream url could not be reached: dns lookup failed",
+		},
+		{
+			name: "connection refused",
+			err:  &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED},
+			want: "the upstream url could not be reached: connection refused",
+		},
+		{
+			name: "host unreachable",
+			err:  &net.OpError{Op: "dial", Net: "tcp", Err: syscall.EHOSTUNREACH},
+			want: "the upstream url could not be reached: host unreachable",
+		},
+		{
+			name: "timeout",
+			err:  fmt.Errorf("dial: %w", os.ErrDeadlineExceeded),
+			want: "the upstream url could not be reached: connection timed out",
+		},
+		{
+			name: "context deadline",
+			err:  fmt.Errorf("dial: %w", context.DeadlineExceeded),
+			want: "the upstream url could not be reached: connection timed out",
+		},
+		{
+			name: "untrusted certificate",
+			err:  &tls.CertificateVerificationError{},
+			want: "the upstream url could not be reached: tls certificate not trusted",
+		},
+		{
+			name: "plaintext service on an https url",
+			err:  tls.RecordHeaderError{Msg: "first record does not look like a TLS handshake"},
+			want: "the upstream url could not be reached: not a tls endpoint",
+		},
+		{
+			// Nothing we recognise. Better to say only that it could not be
+			// reached than to paste a Go error into the provider form.
+			name: "cause we do not recognise",
+			err:  errors.New("something went sideways"),
+			want: "the upstream url could not be reached",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			wrapped := &modeldiscovery.UnreachableError{Provider: "OpenAI", Err: tc.err}
+			got, blocking := credentialCheckFailure(wrapped)
+			require.True(t, blocking, "an unreachable upstream must block the write")
+			require.Equal(t, tc.want, got)
+		})
+	}
+}
+
+// TestCredentialCheckFailure_AnAnsweringUrlThatIsNotTheApi covers the case a
+// status probe would wave through: the host is up, the credential was accepted
+// or not required, and the body is a login page. Reusing the discovery parser
+// for the check is what catches it.
+func TestCredentialCheckFailure_AnAnsweringUrlThatIsNotTheApi(t *testing.T) {
+	err := fmt.Errorf("%w: decode model listing: unexpected token", modeldiscovery.ErrUnparseableListing)
+
+	got, blocking := credentialCheckFailure(err)
+	require.True(t, blocking)
+	require.Equal(t, "the upstream url answered, but not with a model listing", got)
+}
+
+// TestCredentialCheckFailure_WhatCannotBeCheckedIsNotAFailure pins the
+// difference between "this record is wrong" and "we have no way to ask". A
+// gateway with no listing endpoint, a Bedrock record pointed at a proxy, and a
+// self-hosted endpoint the proxy reaches through the tunnel are all legitimate
+// providers. Blocking them would make the feature a lockout.
+func TestCredentialCheckFailure_WhatCannotBeCheckedIsNotAFailure(t *testing.T) {
+	cases := map[string]error{
+		"no listing endpoint":  modeldiscovery.ErrNoDiscovery,
+		"no derivable host":    fmt.Errorf("%w: %w: bedrock", modeldiscovery.ErrInvalidRequest, modeldiscovery.ErrNoDiscoveryHost),
+		"private upstream":     fmt.Errorf("%w: %w: 10.0.0.5", modeldiscovery.ErrInvalidRequest, modeldiscovery.ErrPrivateHost),
+		"private at dial time": fmt.Errorf("%w: discovery refused to dial non-public address 10.0.0.5", modeldiscovery.ErrPrivateHost),
+		"wrapped by unreachable": &modeldiscovery.UnreachableError{
+			Provider: "vLLM",
+			Err:      fmt.Errorf("%w: dial", modeldiscovery.ErrPrivateHost),
+		},
+	}
+
+	for name, err := range cases {
+		t.Run(name, func(t *testing.T) {
+			message, blocking := credentialCheckFailure(err)
+			require.False(t, blocking, "a provider we cannot check must still save")
+			require.Empty(t, message)
+		})
+	}
+}
+
+// TestCredentialCheckFailure_AnUnrecognisedFailureStillBlocks covers a fault of
+// ours rather than the vendor's — a malformed request this code built, or a
+// catalog entry whose parser does not match its endpoint. The record went
+// unverified either way, and silently saving what we could not check is the
+// thing this feature exists to prevent.
+func TestCredentialCheckFailure_AnUnrecognisedFailureStillBlocks(t *testing.T) {
+	message, blocking := credentialCheckFailure(errors.New("no parser for listing shape \"\""))
+	require.True(t, blocking)
+	require.Equal(t, "the provider could not be checked", message)
+}
+
+// newCheckedProvider returns a record shaped the way the handler guarantees
+// one: a known catalog id, a public upstream and a key.
+func newCheckedProvider(accountID string) *types.Provider {
+	provider := types.NewProvider(accountID)
+	provider.ProviderID = "openai_api"
+	provider.Name = "openai"
+	provider.UpstreamURL = "https://api.openai.com"
+	provider.APIKey = "sk-good"
+	provider.Enabled = true
+	return provider
+}
+
+// TestCreateProvider_RefusesARecordTheVendorRejects is the whole point of the
+// feature: a key with a character missing used to save cleanly and surface
+// minutes later as a failed request with nothing pointing back at the record.
+func TestCreateProvider_RefusesARecordTheVendorRejects(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.vendor.err = &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 401}
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	_, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "the provider rejected the credential")
+
+	var sErr *status.Error
+	require.ErrorAs(t, err, &sErr)
+	require.Equal(t, status.InvalidArgument, sErr.Type(), "the refusal must reach the caller as a 422")
+
+	stored, err := f.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, "account1")
+	require.NoError(t, err)
+	require.Empty(t, stored, "a record that failed its check must not be written")
+}
+
+// TestCreateProvider_ChecksTheCredentialItWasGiven pins what the vendor is
+// asked with, since a check run against the wrong upstream or a stale key
+// would pass while proving nothing.
+func TestCreateProvider_ChecksTheCredentialItWasGiven(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	_, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+	require.NoError(t, err)
+
+	asked := f.vendor.only(t)
+	require.Equal(t, "openai_api", asked.CatalogID)
+	require.Equal(t, "https://api.openai.com", asked.UpstreamURL)
+	require.Equal(t, "sk-good", asked.APIKey)
+}
+
+// TestUpdateProvider_AUrlOnlyChangeIsCheckedAgainstTheStoredKey covers the
+// case that shaped where the check sits. The key never returns to the browser,
+// so an operator editing only the URL has none to offer — the stored one is
+// the only credential there is, and the new URL still has to be proven with
+// it.
+func TestUpdateProvider_AUrlOnlyChangeIsCheckedAgainstTheStoredKey(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+	require.NoError(t, err)
+	f.vendor.requests = nil
+
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Update, true)
+	edit := newCheckedProvider("account1")
+	edit.ID = created.ID
+	edit.UpstreamURL = "https://gateway.example.com"
+	edit.APIKey = "" // the form sends no key when it was not retyped
+
+	_, err = f.manager.UpdateProvider(ctx, "user1", edit)
+	require.NoError(t, err)
+
+	asked := f.vendor.only(t)
+	require.Equal(t, "https://gateway.example.com", asked.UpstreamURL, "the new url must be what gets tested")
+	require.Equal(t, "sk-good", asked.APIKey, "and the stored key must be what tests it")
+}
+
+// TestUpdateProvider_AFailedRotationLeavesTheWorkingKeyInPlace is the
+// half-applied state the check must never produce: refusing the new key while
+// having already replaced the old one would take the provider down.
+func TestUpdateProvider_AFailedRotationLeavesTheWorkingKeyInPlace(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+	require.NoError(t, err)
+
+	f.vendor.err = &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 403}
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Update, true)
+	rotation := newCheckedProvider("account1")
+	rotation.ID = created.ID
+	rotation.APIKey = "sk-typo"
+
+	_, err = f.manager.UpdateProvider(ctx, "user1", rotation)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "the provider rejected the credential")
+
+	stored, err := f.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, "account1", created.ID)
+	require.NoError(t, err)
+	require.Equal(t, "sk-good", stored.APIKey, "the rejected key must not have replaced the working one")
+}
+
+// TestUpdateProvider_AnEditTouchingNeitherFieldAsksNoVendor keeps renames,
+// model rows and price edits off the vendor's doorstep. They have nothing new
+// to prove, and making them wait on a vendor — or fail because one is having a
+// bad day — would be a tax on edits that carry no risk.
+func TestUpdateProvider_AnEditTouchingNeitherFieldAsksNoVendor(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+	require.NoError(t, err)
+	f.vendor.requests = nil
+	// Any call at all now would fail the update, which is what makes the
+	// assertion below load-bearing rather than decorative.
+	f.vendor.err = &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 500}
+
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Update, true)
+	rename := newCheckedProvider("account1")
+	rename.ID = created.ID
+	rename.Name = "openai-renamed"
+	rename.APIKey = ""
+
+	_, err = f.manager.UpdateProvider(ctx, "user1", rename)
+	require.NoError(t, err, "an edit that changes neither url nor key must not be checked")
+	require.Zero(t, f.vendor.calls(), "and must not reach the vendor at all")
+}
+
+// TestCreateProvider_AProviderWeCannotCheckStillSaves covers the eleven
+// catalog entries with no listing endpoint, a Bedrock record behind a proxy,
+// and a self-hosted endpoint on a private network. None of those are evidence
+// the record is wrong, and refusing them would make this a lockout.
+func TestCreateProvider_AProviderWeCannotCheckStillSaves(t *testing.T) {
+	cases := map[string]error{
+		"gateway with no listing endpoint": modeldiscovery.ErrNoDiscovery,
+		"bedrock behind a proxy":           fmt.Errorf("%w: %w", modeldiscovery.ErrInvalidRequest, modeldiscovery.ErrNoDiscoveryHost),
+		"self-hosted on a private network": fmt.Errorf("%w: %w", modeldiscovery.ErrInvalidRequest, modeldiscovery.ErrPrivateHost),
+	}
+
+	for name, vendorErr := range cases {
+		t.Run(name, func(t *testing.T) {
+			ctx := context.Background()
+			f := newBootstrapFixture(t)
+			f.vendor.err = vendorErr
+			f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+			created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+			require.NoError(t, err)
+			require.NotNil(t, created)
+
+			stored, err := f.store.GetAccountAgentNetworkProviders(ctx, store.LockingStrengthNone, "account1")
+			require.NoError(t, err)
+			require.Len(t, stored, 1, "a provider we cannot check must still be written")
+		})
+	}
+}

management/internals/modules/agentnetwork/manager.go

@@ -126,13 +126,15 @@ type managerImpl struct {
 	proxyController    proxy.Controller
 
 	// modelDiscovery queries vendors for the models a credential can reach.
-	// A field rather than a package call so tests can drive it without
-	// reaching the network.
+	// An interface rather than the concrete client because it is now on a
+	// write path: the credential check runs inside CreateProvider and
+	// UpdateProvider, so every test that saves a provider would otherwise
+	// reach a vendor over the network to do it.
 	//
 	// One instance serves every request for the process's lifetime, so its
 	// fields must stay read-only after construction: lazy initialisation
 	// inside Fetch or httpClient would race across request goroutines.
-	modelDiscovery *modeldiscovery.Client
+	modelDiscovery ModelLister
 
 	// reconcileCache holds the last set of synthesised proxy mappings
 	// per account, each paired with the proxy that served it, so a change
@@ -146,6 +148,19 @@ type managerImpl struct {
 	labelRng   *rand.Rand
 }
 
+// ManagerOption replaces a manager dependency at construction. Production
+// passes none; each option exists for something a test cannot let run for
+// real.
+type ManagerOption func(*managerImpl)
+
+// WithModelLister replaces the vendor call behind the provider credential
+// check. A test that saves a provider needs this — the check runs inside
+// CreateProvider and UpdateProvider, so the write path reaches a vendor
+// without it.
+func WithModelLister(lister ModelLister) ManagerOption {
+	return func(m *managerImpl) { m.modelDiscovery = lister }
+}
+
 // NewManager constructs the persistent Agent Network manager. The
 // manager persists provider/policy/guardrail configuration and, on
 // every mutation, reconciles the in-memory synthesised reverse-proxy
@@ -156,8 +171,9 @@ func NewManager(
 	permissionsManager permissions.Manager,
 	accountManager account.Manager,
 	proxyController proxy.Controller,
+	opts ...ManagerOption,
 ) Manager {
-	return &managerImpl{
+	m := &managerImpl{
 		store:              store,
 		accountManager:     accountManager,
 		permissionsManager: permissionsManager,
@@ -166,6 +182,10 @@ func NewManager(
 		reconcileCache:     make(map[string]map[string]syntheticMapping),
 		labelRng:           rand.New(rand.NewSource(time.Now().UnixNano())),
 	}
+	for _, opt := range opts {
+		opt(m)
+	}
+	return m
 }
 
 func (m *managerImpl) GetAllProviders(ctx context.Context, accountID, userID string) ([]*types.Provider, error) {
@@ -230,6 +250,13 @@ func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provide
 		return nil, status.Errorf(status.InvalidArgument, "api_key is required when creating an agent network provider")
 	}
 
+	// Before anything is persisted: a record whose upstream or credential does
+	// not work is rejected here rather than discovered later as a failed
+	// request with nothing pointing back at it.
+	if err := m.checkProviderCredential(ctx, provider); err != nil {
+		return nil, err
+	}
+
 	if provider.ID == "" {
 		fresh := types.NewProvider(provider.AccountID)
 		provider.ID = fresh.ID
@@ -269,6 +296,20 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide
 	} else if strings.TrimSpace(provider.APIKey) == "" {
 		return nil, status.Errorf(status.InvalidArgument, "api_key must be non-blank when rotating an agent network provider")
 	}
+
+	// Only the two fields the vendor would judge are worth a round-trip. This
+	// same call carries renames, model rows and price edits, and none of those
+	// should wait on a vendor — or be refused because one is having a bad day.
+	//
+	// The comparison runs after the merge above, so an update that changes only
+	// the URL reads as unchanged on the key and is checked against the stored
+	// one, which is the only credential the operator has to offer here.
+	if provider.UpstreamURL != existing.UpstreamURL || provider.APIKey != existing.APIKey {
+		if err := m.checkProviderCredential(ctx, provider); err != nil {
+			return nil, err
+		}
+	}
+
 	// Always preserve the session keypair across updates so existing
 	// session cookies stay valid. The keys are server-managed and
 	// never surfaced through the API.

management/internals/modules/agentnetwork/modeldiscovery/discovery.go

@@ -146,7 +146,7 @@ func (c *Client) Fetch(ctx context.Context, req Request) ([]Model, error) {
 
 	resp, err := c.httpClient().Do(httpReq)
 	if err != nil {
-		return nil, fmt.Errorf("reach %s: %w", entry.Name, err)
+		return nil, &UnreachableError{Provider: entry.Name, Err: err}
 	}
 	defer func() { _ = resp.Body.Close() }()
 
@@ -157,7 +157,7 @@ func (c *Client) Fetch(ctx context.Context, req Request) ([]Model, error) {
 	if resp.StatusCode != http.StatusOK {
 		// Surface the vendor's own status. An operator whose key lacks a scope
 		// needs to see 403 rather than a generic failure.
-		return nil, fmt.Errorf("%s returned %d for its model listing", entry.Name, resp.StatusCode)
+		return nil, &VendorStatusError{Provider: entry.Name, Status: resp.StatusCode}
 	}
 
 	ids, err := parseListing(entry.Discovery.Shape, body)
@@ -194,8 +194,8 @@ func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, erro
 			region = RegionFromUpstream(entry, req.UpstreamURL)
 		}
 		if region == "" {
-			return "", fmt.Errorf("%w: %s discovery needs a region, and none could be read from the provider upstream",
-				ErrInvalidRequest, entry.Name)
+			return "", fmt.Errorf("%w: %w: %s discovery needs a region, and none could be read from the provider upstream",
+				ErrInvalidRequest, ErrNoDiscoveryHost, entry.Name)
 		}
 		host = strings.ReplaceAll(host, catalog.RegionPlaceholder, region)
 	}
@@ -266,7 +266,7 @@ func (c *Client) checkPublicHost(host string) error {
 	// loopback address is still a way to reach loopback.
 	for _, addr := range addrs {
 		if !isPublic(addr) {
-			return fmt.Errorf("%w: discovery host %q resolves to a non-public address", ErrInvalidRequest, host)
+			return fmt.Errorf("%w: %w: discovery host %q resolves to a non-public address", ErrInvalidRequest, ErrPrivateHost, host)
 		}
 	}
 	return nil
@@ -463,7 +463,7 @@ func guardDialAddress(address string) error {
 		return fmt.Errorf("discovery dial address %q is not an IP", host)
 	}
 	if !isPublic(addr) {
-		return fmt.Errorf("discovery refused to dial non-public address %s", addr)
+		return fmt.Errorf("%w: discovery refused to dial non-public address %s", ErrPrivateHost, addr)
 	}
 	return nil
 }

management/internals/modules/agentnetwork/modeldiscovery/failure.go

@@ -0,0 +1,117 @@
+package modeldiscovery
+
+import (
+	"context"
+	"crypto/tls"
+	"errors"
+	"fmt"
+	"net"
+	"os"
+	"syscall"
+)
+
+// The errors below exist so a caller can tell one discovery failure from
+// another without reading the message. Fetch is used for two jobs now: filling
+// the model picker, which only needs to know that it failed, and checking a
+// provider's credential at save time, which has to tell the operator whether
+// the URL or the key is the problem. A string is the wrong thing to branch on
+// for the second, so each failure carries its own type.
+
+// VendorStatusError reports that the vendor answered the listing with
+// something other than 200. Status is the vendor's own code: 401 and 403 mean
+// the credential was refused, 404 and 405 mean the URL does not serve this
+// API at all, and 5xx means the vendor is unwell — three different things to
+// tell an operator, and only the number separates them.
+type VendorStatusError struct {
+	Provider string
+	Status   int
+}
+
+func (e *VendorStatusError) Error() string {
+	return fmt.Sprintf("%s returned %d for its model listing", e.Provider, e.Status)
+}
+
+// UnreachableError reports that the request never reached the vendor at all:
+// the name did not resolve, the connection was refused, TLS failed, or the
+// attempt timed out. Nothing was authenticated, so the credential is not
+// implicated — only the URL is.
+type UnreachableError struct {
+	Provider string
+	Err      error
+}
+
+func (e *UnreachableError) Error() string {
+	return fmt.Sprintf("reach %s: %v", e.Provider, e.Err)
+}
+
+func (e *UnreachableError) Unwrap() error { return e.Err }
+
+// Reason names the transport failure in the words an operator can act on.
+// "connection refused" and "no such host" are the difference between a wrong
+// port and a wrong hostname, which is worth the few lines it takes to tell
+// them apart. An empty string means the cause was not one we recognise, and
+// the caller should say only that the host could not be reached rather than
+// paste a Go error into the UI.
+func (e *UnreachableError) Reason() string {
+	err := e.Err
+
+	var dns *net.DNSError
+	if errors.As(err, &dns) {
+		if dns.IsNotFound {
+			return "no such host"
+		}
+		return "dns lookup failed"
+	}
+
+	// Timeouts are checked before the syscall cases: a dial that times out is
+	// reported as a net.OpError wrapping a timeout, and the operator needs to
+	// hear "timed out" rather than the syscall underneath it.
+	if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, os.ErrDeadlineExceeded) {
+		return "connection timed out"
+	}
+	var netErr net.Error
+	if errors.As(err, &netErr) && netErr.Timeout() {
+		return "connection timed out"
+	}
+
+	if errors.Is(err, syscall.ECONNREFUSED) {
+		return "connection refused"
+	}
+	if errors.Is(err, syscall.EHOSTUNREACH) || errors.Is(err, syscall.ENETUNREACH) {
+		return "host unreachable"
+	}
+
+	var certErr *tls.CertificateVerificationError
+	if errors.As(err, &certErr) {
+		return "tls certificate not trusted"
+	}
+	var recordErr tls.RecordHeaderError
+	if errors.As(err, &recordErr) {
+		return "not a tls endpoint"
+	}
+
+	return ""
+}
+
+// ErrUnparseableListing marks a 200 whose body is not a model listing in the
+// shape the catalog declared. It is a distinct outcome from a status refusal:
+// the host answered and authenticated fine, it just is not the API we were
+// aiming at — a login page or an unrelated service on the configured URL.
+var ErrUnparseableListing = errors.New("response is not a model listing")
+
+// ErrNoDiscoveryHost marks a provider whose listing host cannot be worked out
+// from the record. Bedrock's listing lives on a control-plane host derived
+// from the region in the upstream, so an operator pointing the record at a
+// proxied or self-hosted endpoint leaves nowhere to send it. Inventing a host
+// would spend their credential somewhere they never configured.
+//
+// It wraps ErrInvalidRequest so the discovery endpoint keeps answering 400,
+// while a credential check can recognise it as "cannot be checked" rather
+// than "is broken".
+var ErrNoDiscoveryHost = errors.New("provider has no derivable discovery host")
+
+// ErrPrivateHost marks an upstream that resolves somewhere the management
+// server will not dial. A self-hosted vendor endpoint on a private network is
+// a legitimate provider — the proxy reaches it through the tunnel — so this
+// means the check cannot run, not that the record is wrong.
+var ErrPrivateHost = errors.New("discovery host is not publicly routable")

management/internals/modules/agentnetwork/modeldiscovery/parse.go

@@ -43,7 +43,7 @@ func parseOpenAIData(body []byte) ([]listedModel, error) {
 		} `json:"data"`
 	}
 	if err := json.Unmarshal(body, &doc); err != nil {
-		return nil, fmt.Errorf("decode model listing: %w", err)
+		return nil, fmt.Errorf("%w: decode model listing: %w", ErrUnparseableListing, err)
 	}
 	out := make([]listedModel, 0, len(doc.Data))
 	for _, entry := range doc.Data {
@@ -71,7 +71,7 @@ func parseBedrockInferenceProfiles(body []byte) ([]listedModel, error) {
 		} `json:"inferenceProfileSummaries"`
 	}
 	if err := json.Unmarshal(body, &doc); err != nil {
-		return nil, fmt.Errorf("decode inference-profile listing: %w", err)
+		return nil, fmt.Errorf("%w: decode inference-profile listing: %w", ErrUnparseableListing, err)
 	}
 	out := make([]listedModel, 0, len(doc.Summaries))
 	for _, entry := range doc.Summaries {
@@ -98,7 +98,7 @@ func parseVertexPublisherModels(body []byte) ([]listedModel, error) {
 		} `json:"publisherModels"`
 	}
 	if err := json.Unmarshal(body, &doc); err != nil {
-		return nil, fmt.Errorf("decode publisher-model listing: %w", err)
+		return nil, fmt.Errorf("%w: decode publisher-model listing: %w", ErrUnparseableListing, err)
 	}
 	out := make([]listedModel, 0, len(doc.Models))
 	for _, entry := range doc.Models {

management/internals/modules/agentnetwork/settings_bootstrap_test.go

@@ -26,6 +26,10 @@ type bootstrapFixture struct {
 	manager Manager
 	store   store.Store
 	perms   *permissions.MockManager
+	// vendor stands in for the provider credential check's vendor call, which
+	// runs on every provider write. Without it these tests would reach a real
+	// vendor to save a record.
+	vendor *stubLister
 }
 
 func newBootstrapFixture(t *testing.T) *bootstrapFixture {
@@ -47,10 +51,12 @@ func newBootstrapFixture(t *testing.T) *bootstrapFixture {
 	accounts.EXPECT().UpdateAccountPeers(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 	accounts.EXPECT().BufferUpdateAccountPeers(gomock.Any(), gomock.Any(), gomock.Any()).AnyTimes()
 
+	vendor := &stubLister{}
 	return &bootstrapFixture{
-		manager: NewManager(st, perms, accounts, nil),
+		manager: NewManager(st, perms, accounts, nil, WithModelLister(vendor)),
 		store:   st,
 		perms:   perms,
+		vendor:  vendor,
 	}
 }
 
@@ -211,6 +217,7 @@ func TestCreateProviderHasNoSettingsSideEffects(t *testing.T) {
 	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
 
 	provider := types.NewProvider("account1")
+	provider.ProviderID = "openai_api"
 	provider.Name = "openai"
 	provider.UpstreamURL = "https://api.openai.com"
 	provider.APIKey = "sk-test"