From 4e3578b84728048bd395c47d4e76f18b0eb25e5e Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Sun, 23 Aug 2026 21:26:06 +0000
Subject: [PATCH 01/16] [management] Check a provider's url and credential
 before saving it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
 .../modules/agentnetwork/credentialcheck.go   | 120 ++++++
 .../agentnetwork/credentialcheck_test.go      | 384 ++++++++++++++++++
 .../internals/modules/agentnetwork/manager.go |  49 ++-
 .../agentnetwork/modeldiscovery/discovery.go  |  12 +-
 .../agentnetwork/modeldiscovery/failure.go    | 117 ++++++
 .../agentnetwork/modeldiscovery/parse.go      |   6 +-
 .../agentnetwork/settings_bootstrap_test.go   |   9 +-
 7 files changed, 683 insertions(+), 14 deletions(-)
 create mode 100644 management/internals/modules/agentnetwork/credentialcheck.go
 create mode 100644 management/internals/modules/agentnetwork/credentialcheck_test.go
 create mode 100644 management/internals/modules/agentnetwork/modeldiscovery/failure.go

diff --git a/management/internals/modules/agentnetwork/credentialcheck.go b/management/internals/modules/agentnetwork/credentialcheck.go
new file mode 100644
index 00000000000..dd62aaed427
--- /dev/null
+++ b/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
+}
diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
new file mode 100644
index 00000000000..d167a85a67f
--- /dev/null
+++ b/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")
+		})
+	}
+}
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index 41789195e52..34e5c322087 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/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.
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
index 253cc63b3f4..42534eadb25 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
+++ b/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
 }
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/failure.go b/management/internals/modules/agentnetwork/modeldiscovery/failure.go
new file mode 100644
index 00000000000..89da0a40da2
--- /dev/null
+++ b/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")
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/parse.go b/management/internals/modules/agentnetwork/modeldiscovery/parse.go
index 83048cb8a90..67a10bf3642 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/parse.go
+++ b/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 {
diff --git a/management/internals/modules/agentnetwork/settings_bootstrap_test.go b/management/internals/modules/agentnetwork/settings_bootstrap_test.go
index fc6fd8b82df..fea62353ea0 100644
--- a/management/internals/modules/agentnetwork/settings_bootstrap_test.go
+++ b/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"

From 264ca31bf2bff54457a72c4f6e9fefa934d76181 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Mon, 24 Aug 2026 05:57:13 +0000
Subject: [PATCH 02/16] [management] Trim the credential-check comments to the
 repo budget

---
 .../modules/agentnetwork/credentialcheck.go   | 58 +++++-----------
 .../agentnetwork/modeldiscovery/failure.go    | 66 ++++++++-----------
 2 files changed, 46 insertions(+), 78 deletions(-)

diff --git a/management/internals/modules/agentnetwork/credentialcheck.go b/management/internals/modules/agentnetwork/credentialcheck.go
index dd62aaed427..a49cc1d419a 100644
--- a/management/internals/modules/agentnetwork/credentialcheck.go
+++ b/management/internals/modules/agentnetwork/credentialcheck.go
@@ -12,37 +12,20 @@ import (
 	"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.
+// test that saves a provider would reach a vendor 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.
+// checkProviderCredential refuses a record whose upstream or credential the
+// vendor will not accept.
 //
-// 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.
+// It reuses the discovery Fetch rather than a lighter status probe so it
+// exercises the path the model picker takes: a URL answering 200 with a login
+// page fails here instead of producing an empty picker later.
 func (m *managerImpl) checkProviderCredential(ctx context.Context, provider *types.Provider) error {
 	_, err := m.modelDiscovery.Fetch(ctx, modeldiscovery.Request{
 		CatalogID:   provider.ProviderID,
@@ -59,10 +42,8 @@ func (m *managerImpl) checkProviderCredential(ctx context.Context, provider *typ
 		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.
+	// WriteError logs only what we return, and that carries no status code,
+	// so the vendor's number is recorded here or nowhere.
 	log.WithContext(ctx).Infof("agent network provider %s failed its credential check: %v", provider.ProviderID, err)
 
 	return status.Errorf(status.InvalidArgument, "%s", message)
@@ -71,12 +52,12 @@ func (m *managerImpl) checkProviderCredential(ctx context.Context, provider *typ
 // 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.
+// The strings survive WriteError lowercasing them, and never echo the
+// operator's URL: paths are case-sensitive, so an echoed URL comes back
+// altered and describes 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.
+	// Not checkable. The record may be perfectly good and we have no way to
+	// ask, so reporting a failure would be a guess.
 	switch {
 	case errors.Is(err, modeldiscovery.ErrNoDiscovery),
 		errors.Is(err, modeldiscovery.ErrNoDiscoveryHost),
@@ -92,10 +73,8 @@ func credentialCheckFailure(err error) (message string, blocking bool) {
 		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.
+			// 5xx and 429 included: an outage still leaves the record
+			// unverified, which is what this refuses to save.
 			return "the provider returned an error", true
 		}
 	}
@@ -112,9 +91,8 @@ func credentialCheckFailure(err error) (message string, blocking bool) {
 		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.
+	// Ours rather than the vendor's — a request this code built badly, or a
+	// catalog entry that does not match its parser. Still unverified, so it
+	// still blocks.
 	return "the provider could not be checked", true
 }
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/failure.go b/management/internals/modules/agentnetwork/modeldiscovery/failure.go
index 89da0a40da2..2b351d75f01 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/failure.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/failure.go
@@ -10,18 +10,14 @@ import (
 	"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.
+// Fetch serves two callers with different needs: the model picker, which only
+// needs to know it failed, and the provider credential check, which has to
+// tell an operator whether the URL or the key is at fault. Each failure
+// carries a type so the second does not have to branch on a message.
+
+// VendorStatusError reports a listing answered with something other than 200.
+// Only the vendor's own code separates a refused credential (401, 403) from a
+// URL that does not serve this API (404, 405) from an unwell vendor (5xx).
 type VendorStatusError struct {
 	Provider string
 	Status   int
@@ -31,10 +27,9 @@ 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.
+// UnreachableError reports that the request never reached the vendor: the
+// name did not resolve, the connection was refused, TLS failed, or it timed
+// out. Nothing was authenticated, so only the URL is implicated.
 type UnreachableError struct {
 	Provider string
 	Err      error
@@ -46,12 +41,10 @@ func (e *UnreachableError) Error() string {
 
 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.
+// Reason names the transport failure in words an operator can act on: a wrong
+// port and a wrong hostname fail differently and are worth telling apart.
+// Empty means unrecognised, 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
 
@@ -93,25 +86,22 @@ func (e *UnreachableError) Reason() string {
 	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.
+// ErrUnparseableListing marks a 200 whose body is not a listing in the shape
+// the catalog declared. Distinct from a status refusal: the host answered and
+// authenticated fine, it is just not the API — a login page, say.
 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.
+// ErrNoDiscoveryHost marks a provider whose listing host cannot be derived
+// from the record: Bedrock's control-plane host comes from the region in the
+// upstream, so a proxied endpoint leaves nowhere to send it, and inventing one
+// would spend the credential somewhere 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".
+// Wraps ErrInvalidRequest so the discovery endpoint still answers 400, while a
+// credential check can read it as "cannot be checked" rather than "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.
+// ErrPrivateHost marks an upstream resolving somewhere management will not
+// dial. A self-hosted endpoint on a private network is a legitimate provider
+// the proxy reaches 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")

From 8643faeed57c14d5b2e54308f349f7653c1bf4bd Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Mon, 24 Aug 2026 06:08:16 +0000
Subject: [PATCH 03/16] [management] Document the provider credential check,
 and prove it live
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The OpenAPI change is the 422 both provider routes can now answer, plus what
decides it: the create path checks the pair before storing, the update path
checks only when the upstream or the key moved, and an update omitting the key
is checked against the stored one.

The live tests cover what a unit test structurally cannot. Mocked refusals
prove the classifier maps a status to a message; they cannot show that these
vendors refuse a bad key on their listing endpoint at all, which is the
assumption the feature rests on. The good-key case earns its place beside the
bad one — a check that refused everything would satisfy a test asserting only
the refusal.

The rotation case pins the state worth the most: after a rejected key, an edit
that reuses the stored one still passes. The API never returns a key, so that
is the only way to show the working credential is still there.
---
 .../credential_check_live_test.go             | 204 ++++++++++++++++++
 shared/management/http/api/openapi.yml        |  14 +-
 2 files changed, 216 insertions(+), 2 deletions(-)
 create mode 100644 e2e/agentnetwork/credential_check_live_test.go

diff --git a/e2e/agentnetwork/credential_check_live_test.go b/e2e/agentnetwork/credential_check_live_test.go
new file mode 100644
index 00000000000..f859cb329dc
--- /dev/null
+++ b/e2e/agentnetwork/credential_check_live_test.go
@@ -0,0 +1,204 @@
+//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.
+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.
+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")
+}
+
+// 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.
+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.
+	renamed := credentialProviderRequest(tc, "e2e-cred-rotate-renamed", "")
+	updated, err := srv.UpdateProvider(ctx, prov.Id, renamed)
+	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)
+}
diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index 3ab5a2e428e..c318c8b0494 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -14143,7 +14143,10 @@ paths:
           "$ref": "#/components/responses/internal_error"
     post:
       summary: Create an Agent Network Provider
-      description: Connects a new Agent Network AI provider for the account.
+      description: |
+        Connects a new Agent Network AI provider for the account.
+
+        The upstream URL and credential are checked against the vendor before the provider is stored, so a record that cannot reach its vendor is refused rather than saved. Returns 422 with a message naming whichever of the two is at fault. A catalog provider with no listing endpoint, and one whose upstream the check cannot reach from the management service, are stored without being checked.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]
@@ -14169,6 +14172,8 @@ paths:
           "$ref": "#/components/responses/forbidden"
         '409':
           "$ref": "#/components/responses/conflict"
+        '422':
+          "$ref": "#/components/responses/validation_failed_simple"
         '500':
           "$ref": "#/components/responses/internal_error"
   /api/agent-network/providers/{providerId}:
@@ -14205,7 +14210,10 @@ paths:
           "$ref": "#/components/responses/internal_error"
     put:
       summary: Update an Agent Network Provider
-      description: Update an existing Agent Network AI provider.
+      description: |
+        Update an existing Agent Network AI provider.
+
+        When the upstream URL or the API key changes, the pair is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. An update omitting the API key is checked against the stored one. Edits touching neither field are stored without a check.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]
@@ -14240,6 +14248,8 @@ paths:
           "$ref": "#/components/responses/not_found"
         '409':
           "$ref": "#/components/responses/conflict"
+        '422':
+          "$ref": "#/components/responses/validation_failed_simple"
         '500':
           "$ref": "#/components/responses/internal_error"
     delete:

From 6c7a6c3fb8d45d9d0c1c8d1718ac8e7dcbb63e0f Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Mon, 24 Aug 2026 07:22:20 +0000
Subject: [PATCH 04/16] [management] Classify a host that will not resolve as
 unreachable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The SSRF guard resolves the host before any request is built, so a name that
does not resolve fails there rather than at the transport, and that error
reached the caller unclassified — an operator with a typo in the hostname was
told the provider could not be checked rather than that the url could not be
reached. It is the commonest way for an upstream to be wrong.

The live suite is what caught it: the unit tests construct the transport
errors directly and so never went through the guard.

The management fixture moves to a private upstream in the same change. It
wants a provider row to hang a policy off, not a working vendor, and it was
pointing a dummy key at the real api.openai.com — which the credential check
now correctly refuses. A private address is left unchecked whether or not the
run has vendor keys, and covers that path while it is there.
---
 e2e/agentnetwork/management_test.go           | 12 +++++--
 .../agentnetwork/modeldiscovery/discovery.go  | 10 +++++-
 .../modeldiscovery/discovery_test.go          | 31 +++++++++++++++++++
 3 files changed, 49 insertions(+), 4 deletions(-)

diff --git a/e2e/agentnetwork/management_test.go b/e2e/agentnetwork/management_test.go
index 9e3176d68e2..6868a9d4d9c 100644
--- a/e2e/agentnetwork/management_test.go
+++ b/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)
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
index 42534eadb25..b3f96c0a4e2 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
@@ -202,7 +202,15 @@ func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, erro
 
 	target := &url.URL{Scheme: "https", Host: host, Path: entry.Discovery.Path, RawQuery: entry.Discovery.Query}
 	if err := c.checkPublicHost(target.Hostname()); err != nil {
-		return "", err
+		// A host that refuses to resolve fails here, before any request is
+		// built, and it is the commonest way for an upstream to be wrong. It
+		// has to reach the caller as unreachable rather than as an
+		// unclassified fault. ErrPrivateHost is the other outcome and means
+		// something else entirely — not a bad host, one we decline to dial.
+		if errors.Is(err, ErrPrivateHost) {
+			return "", err
+		}
+		return "", &UnreachableError{Provider: entry.Name, Err: err}
 	}
 	return target.String(), nil
 }
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
index 133bd5148ac..9b39f387e5f 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
@@ -2,7 +2,9 @@ package modeldiscovery
 
 import (
 	"context"
+	"errors"
 	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"net/netip"
@@ -530,3 +532,32 @@ func TestBedrockProfilesFromAnyGeographyArrivePriced(t *testing.T) {
 	// only form that works at invoke time.
 	assert.Equal(t, "jp.anthropic.claude-sonnet-5-20260514-v1:0", models[0].ID)
 }
+
+// TestFetch_AHostThatWillNotResolveIsUnreachable closes a gap the live suite
+// found. The SSRF guard resolves the host before any request is built, so a
+// name that does not resolve fails there rather than at the transport — and
+// that error used to reach the caller unclassified. A wrong hostname is the
+// commonest way for an upstream to be wrong, so it has to arrive as
+// "unreachable" and not as an unrecognised fault.
+func TestFetch_AHostThatWillNotResolveIsUnreachable(t *testing.T) {
+	// A resolver whose dial always fails, so the lookup errors without the
+	// test depending on real DNS.
+	refusing := &net.Resolver{
+		PreferGo: true,
+		Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
+			return nil, errors.New("resolver unavailable")
+		},
+	}
+	client := &Client{Resolver: refusing}
+
+	_, err := client.Fetch(context.Background(), Request{
+		CatalogID:   "openai_api",
+		UpstreamURL: "https://not-a-real-vendor-host.example.invalid",
+		APIKey:      "sk-test",
+	})
+
+	require.Error(t, err)
+	var unreachable *UnreachableError
+	require.ErrorAs(t, err, &unreachable, "a host that will not resolve must classify as unreachable")
+	require.NotErrorIs(t, err, ErrPrivateHost, "it is not a host we declined to dial")
+}

From bfc96ec9b5b074f8522dfe305de602f24960e532 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Mon, 24 Aug 2026 07:59:26 +0000
Subject: [PATCH 05/16] [management] Stop a proxy in the egress path from
 disabling the check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The dial-time guard reported every non-public address as ErrPrivateHost, which
the credential check reads as "this upstream cannot be reached from here, so
save it unchecked". checkPublicHost has already cleared the target by the time
anything is dialled, so an address refused at the socket is never the
operator's upstream — it is a rebinding attempt, or an HTTP proxy the
management server egresses through. A deployment behind such a proxy would
install this feature and have it silently do nothing on every provider.

It now reports an ordinary failure, which classifies as unreachable and blocks.
Only the resolve-stage check still means "cannot be checked", and that one
knows it is looking at the operator's own host.

This is also why the three fixtures below passed locally and failed in CI: a
sandbox that egresses through a loopback proxy skipped the check entirely,
while CI reached the real api.openai.com and had the dummy key refused. They
want a provider row rather than a working vendor, so they move to a private
address and no longer depend on where a hostname resolves or whether the runner
has egress.
---
 .../agentnetwork/credentialcheck_test.go      | 13 +++----
 .../handlers/providers_handler_test.go        |  7 ++--
 .../agentnetwork/modeldiscovery/discovery.go  |  9 ++++-
 .../modeldiscovery/discovery_test.go          | 36 +++++++++++++++++++
 .../agentnetwork_budgetrule_realstack_test.go | 12 ++++---
 .../server/agentnetwork_realstack_test.go     | 12 ++++---
 6 files changed, 69 insertions(+), 20 deletions(-)

diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
index d167a85a67f..3ea85ba812d 100644
--- a/management/internals/modules/agentnetwork/credentialcheck_test.go
+++ b/management/internals/modules/agentnetwork/credentialcheck_test.go
@@ -115,7 +115,7 @@ func TestCredentialCheckFailure_NamesTheTransportFault(t *testing.T) {
 	}{
 		{
 			name: "unknown host",
-			err:  &net.DNSError{Err: "no such host", Name: "api.exmaple.com", IsNotFound: true},
+			err:  &net.DNSError{Err: "no such host", Name: "api.example.com", IsNotFound: true},
 			want: "the upstream url could not be reached: no such host",
 		},
 		{
@@ -191,14 +191,9 @@ func TestCredentialCheckFailure_AnAnsweringUrlThatIsNotTheApi(t *testing.T) {
 // 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),
-		},
+		"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),
 	}
 
 	for name, err := range cases {
diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler_test.go b/management/internals/modules/agentnetwork/handlers/providers_handler_test.go
index 05024cde917..c32835a562f 100644
--- a/management/internals/modules/agentnetwork/handlers/providers_handler_test.go
+++ b/management/internals/modules/agentnetwork/handlers/providers_handler_test.go
@@ -64,10 +64,13 @@ func TestValidate_ModelRates(t *testing.T) {
 func TestProviderHandler_UpdateReplacesFullState(t *testing.T) {
 	f := newAgentNetworkHandlerFixture(t)
 
+	// A private upstream: the save-time credential check leaves it unchecked
+	// rather than spending "sk-test" against the real api.openai.com, which
+	// the vendor refuses.
 	create := `{
         "provider_id": "openai_api",
         "name": "openai",
-        "upstream_url": "https://api.openai.com",
+        "upstream_url": "https://10.255.255.1",
         "api_key": "sk-test",
         "enabled": true,
         "metadata_disabled": true,
@@ -84,7 +87,7 @@ func TestProviderHandler_UpdateReplacesFullState(t *testing.T) {
 
 	// Minimal update: only the required fields, no api_key. Everything
 	// optional must land as its zero value.
-	update := `{"provider_id": "openai_api", "name": "openai-renamed", "upstream_url": "https://api.openai.com", "enabled": true}`
+	update := `{"provider_id": "openai_api", "name": "openai-renamed", "upstream_url": "https://10.255.255.1", "enabled": true}`
 	rec = f.do(t, nethttp.MethodPut, "/agent-network/providers/"+created.Id, update)
 	require.Equal(t, nethttp.StatusOK, rec.Code, "update without api_key must succeed (key is preserved): %s", rec.Body.String())
 
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
index b3f96c0a4e2..21d4656ec21 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
@@ -471,7 +471,14 @@ func guardDialAddress(address string) error {
 		return fmt.Errorf("discovery dial address %q is not an IP", host)
 	}
 	if !isPublic(addr) {
-		return fmt.Errorf("%w: discovery refused to dial non-public address %s", ErrPrivateHost, addr)
+		// Deliberately not ErrPrivateHost, which means "this upstream is on a
+		// private network, so we cannot check it" and lets a save through
+		// unchecked. checkPublicHost has already cleared the target by the
+		// time anything is dialled, so an address refused here is not the
+		// operator's upstream: it is a rebinding attempt, or an HTTP proxy in
+		// the path. Neither may quietly skip the check — one is hostile, and
+		// the other would silently disable this on every provider.
+		return fmt.Errorf("discovery refused to dial non-public address %s", addr)
 	}
 	return nil
 }
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
index 9b39f387e5f..47862b573b5 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
@@ -561,3 +561,39 @@ func TestFetch_AHostThatWillNotResolveIsUnreachable(t *testing.T) {
 	require.ErrorAs(t, err, &unreachable, "a host that will not resolve must classify as unreachable")
 	require.NotErrorIs(t, err, ErrPrivateHost, "it is not a host we declined to dial")
 }
+
+// TestFetch_AProxyInThePathDoesNotSilentlyDisableTheCheck pins a fail-open the
+// dial-time guard can produce. checkPublicHost clears the target before
+// anything is dialled, so a private address refused at the socket is never the
+// operator's upstream — it is a rebinding attempt, or an HTTP proxy the
+// management server egresses through. Reporting either as ErrPrivateHost would
+// read as "this provider cannot be checked" and let every save through
+// unchecked, which is how a proxied deployment would install this feature and
+// have it quietly do nothing.
+func TestFetch_AProxyInThePathDoesNotSilentlyDisableTheCheck(t *testing.T) {
+	// A transport that refuses at the socket exactly as the guard does, with a
+	// loopback address standing in for the proxy the dial went to.
+	client := &Client{HTTPClient: &http.Client{
+		Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
+			return nil, guardDialAddress("127.0.0.1:38599")
+		}),
+		CheckRedirect: refuseRedirect,
+	}}
+
+	_, err := client.Fetch(context.Background(), Request{
+		CatalogID:   "openai_api",
+		UpstreamURL: "https://api.openai.com",
+		APIKey:      "sk-test",
+	})
+
+	require.Error(t, err)
+	require.NotErrorIs(t, err, ErrPrivateHost,
+		"a refusal at the socket must not read as an upstream we cannot check")
+	var unreachable *UnreachableError
+	require.ErrorAs(t, err, &unreachable, "it is the vendor we failed to reach")
+}
+
+// roundTripFunc adapts a function to http.RoundTripper.
+type roundTripFunc func(*http.Request) (*http.Response, error)
+
+func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
diff --git a/management/server/agentnetwork_budgetrule_realstack_test.go b/management/server/agentnetwork_budgetrule_realstack_test.go
index 95f9c35dce3..b046581e816 100644
--- a/management/server/agentnetwork_budgetrule_realstack_test.go
+++ b/management/server/agentnetwork_budgetrule_realstack_test.go
@@ -96,10 +96,14 @@ func TestAgentNetwork_UpdateSettings_PreservesImmutableAndTogglesCollection(t *t
 	assert.False(t, before.EnablePromptCollection, "prompt collection defaults off")
 
 	_, err = mgr.CreateProvider(ctx, adminUserID, &agenttypes.Provider{
-		AccountID:   accountID,
-		ProviderID:  "openai_api",
-		Name:        "openai",
-		UpstreamURL: "https://api.openai.com",
+		AccountID:  accountID,
+		ProviderID: "openai_api",
+		Name:       "openai",
+		// A private address: the save-time credential check leaves it
+		// unchecked rather than spending a dummy key against the real
+		// api.openai.com, which the vendor refuses and which would make
+		// this test depend on the runner having egress.
+		UpstreamURL: "https://10.255.255.1",
 		APIKey:      "sk-test",
 		Enabled:     true,
 		Models:      []agenttypes.ProviderModel{{ID: "gpt-5.4"}},
diff --git a/management/server/agentnetwork_realstack_test.go b/management/server/agentnetwork_realstack_test.go
index d4efb1607c8..d438ffbdd3c 100644
--- a/management/server/agentnetwork_realstack_test.go
+++ b/management/server/agentnetwork_realstack_test.go
@@ -101,10 +101,14 @@ func TestAgentNetwork_ProviderCRUD_FansOutToProxyAndClientPeers(t *testing.T) {
 	drain(proxyCh)
 
 	provider, err := agentMgr.CreateProvider(ctx, adminUserID, &agenttypes.Provider{
-		AccountID:   accountID,
-		ProviderID:  "openai_api",
-		Name:        "openai-test",
-		UpstreamURL: "https://api.openai.com",
+		AccountID:  accountID,
+		ProviderID: "openai_api",
+		Name:       "openai-test",
+		// A private address: the save-time credential check leaves it
+		// unchecked rather than spending a dummy key against the real
+		// api.openai.com, which the vendor refuses and which would make
+		// this test depend on the runner having egress.
+		UpstreamURL: "https://10.255.255.1",
 		APIKey:      "sk-test-key",
 		Enabled:     true,
 		Models:      []agenttypes.ProviderModel{{ID: "gpt-5.4"}},

From b962f99f3a9ddcaa150bff2d68d62bec3169c681 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Mon, 24 Aug 2026 08:59:55 +0000
Subject: [PATCH 06/16] [management] Say what went wrong when loading a
 provider's models fails
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Pressing "Load models from provider" against a key the vendor refuses answered
"internal server error". Every outcome on that path is the operator's own key
or upstream, so a 500 was wrong twice over: it told them the server broke, and
it named nothing they could act on.

The failures are already typed and already have sentences written for them —
the save-time check translates the same set. Discovery now runs them through
the same classifier, so the button reports a refused credential or an
unreachable url in the words the form uses elsewhere.

ErrNoDiscovery and ErrInvalidRequest pass through untouched. The handler maps
both already, and a provider with no listing endpoint is a fact about the
catalog entry rather than a failure — the caller falls back to the catalog's
own models instead of showing an error at all.
---
 .../modules/agentnetwork/credentialcheck.go   | 26 ++++++
 .../agentnetwork/credentialcheck_test.go      | 79 +++++++++++++++++++
 .../internals/modules/agentnetwork/manager.go |  6 +-
 3 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/management/internals/modules/agentnetwork/credentialcheck.go b/management/internals/modules/agentnetwork/credentialcheck.go
index a49cc1d419a..8306e1ee0c0 100644
--- a/management/internals/modules/agentnetwork/credentialcheck.go
+++ b/management/internals/modules/agentnetwork/credentialcheck.go
@@ -49,6 +49,32 @@ func (m *managerImpl) checkProviderCredential(ctx context.Context, provider *typ
 	return status.Errorf(status.InvalidArgument, "%s", message)
 }
 
+// discoveryFailure renders a failed model listing for the operator who pressed
+// the button. Every outcome here is something they did or configured — a key
+// the vendor refused, an upstream that does not answer — so it owes them the
+// same sentence a refused save gives, not the generic 500 an unclassified
+// error turns into.
+//
+// ErrNoDiscovery and ErrInvalidRequest pass through untouched: the handler
+// already maps them, and "this provider has no listing endpoint" is a fact
+// about the catalog rather than a failure to report as one.
+func discoveryFailure(ctx context.Context, catalogID string, err error) error {
+	if errors.Is(err, modeldiscovery.ErrNoDiscovery) || errors.Is(err, modeldiscovery.ErrInvalidRequest) {
+		return err
+	}
+
+	message, _ := credentialCheckFailure(err)
+	if message == "" {
+		return err
+	}
+
+	// The operator's message carries no status code, so the vendor's number is
+	// recorded here or nowhere.
+	log.WithContext(ctx).Infof("agent network model discovery for %s failed: %v", catalogID, 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.
 //
diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
index 3ea85ba812d..dc0cf133cb6 100644
--- a/management/internals/modules/agentnetwork/credentialcheck_test.go
+++ b/management/internals/modules/agentnetwork/credentialcheck_test.go
@@ -377,3 +377,82 @@ func TestCreateProvider_AProviderWeCannotCheckStillSaves(t *testing.T) {
 		})
 	}
 }
+
+// TestDiscoveryFailure_TellsTheOperatorWhatWentWrong covers the button, not the
+// save. Pressing "Load models from provider" against a bad key used to answer
+// "internal server error", which names neither the thing that failed nor
+// anything the operator could act on — every outcome here is their key or their
+// URL.
+func TestDiscoveryFailure_TellsTheOperatorWhatWentWrong(t *testing.T) {
+	cases := map[string]struct {
+		err  error
+		want string
+	}{
+		"refused credential": {
+			err:  &modeldiscovery.VendorStatusError{Provider: "Bedrock", Status: 403},
+			want: "the provider rejected the credential",
+		},
+		"upstream that is not the api": {
+			err:  &modeldiscovery.VendorStatusError{Provider: "OpenAI", Status: 404},
+			want: "the upstream url did not answer a model listing",
+		},
+		"upstream that does not resolve": {
+			err: &modeldiscovery.UnreachableError{
+				Provider: "OpenAI",
+				Err:      &net.DNSError{Err: "no such host", Name: "api.example.com", IsNotFound: true},
+			},
+			want: "the upstream url could not be reached: no such host",
+		},
+		"vendor having a bad day": {
+			err:  &modeldiscovery.VendorStatusError{Provider: "Anthropic", Status: 503},
+			want: "the provider returned an error",
+		},
+	}
+
+	for name, tc := range cases {
+		t.Run(name, func(t *testing.T) {
+			err := discoveryFailure(context.Background(), "openai_api", tc.err)
+			require.EqualError(t, err, tc.want)
+
+			var sErr *status.Error
+			require.ErrorAs(t, err, &sErr)
+			require.Equal(t, status.InvalidArgument, sErr.Type(),
+				"a failure the operator caused must not read as a server fault")
+		})
+	}
+}
+
+// TestDiscoveryFailure_LeavesTheCatalogFactsAlone keeps the two outcomes the
+// handler already maps. A provider with no listing endpoint is a fact about the
+// catalog entry, and the caller falls back to the catalog's own models rather
+// than showing an error at all — rewriting it as a refusal would turn a normal
+// path into one.
+func TestDiscoveryFailure_LeavesTheCatalogFactsAlone(t *testing.T) {
+	for name, err := range map[string]error{
+		"no listing endpoint": modeldiscovery.ErrNoDiscovery,
+		"bad request":         fmt.Errorf("%w: unknown catalog provider", modeldiscovery.ErrInvalidRequest),
+	} {
+		t.Run(name, func(t *testing.T) {
+			require.Equal(t, err, discoveryFailure(context.Background(), "openai_api", err),
+				"the handler's own mapping must still see the original error")
+		})
+	}
+}
+
+// TestDiscoverProviderModels_SurfacesTheVendorRefusal drives the manager rather
+// than the classifier, so a future refactor that stops translating on this path
+// fails here rather than silently going back to 500s.
+func TestDiscoverProviderModels_SurfacesTheVendorRefusal(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.DiscoverProviderModels(ctx, "account1", "user1", modeldiscovery.Request{
+		CatalogID:   "openai_api",
+		UpstreamURL: "https://api.openai.com",
+		APIKey:      "sk-wrong",
+	}, "")
+
+	require.EqualError(t, err, "the provider rejected the credential")
+}
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index 34e5c322087..fe646d55549 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/management/internals/modules/agentnetwork/manager.go
@@ -231,7 +231,11 @@ func (m *managerImpl) DiscoverProviderModels(ctx context.Context, accountID, use
 		req.APIKey = record.APIKey
 	}
 
-	return m.modelDiscovery.Fetch(ctx, req)
+	models, err := m.modelDiscovery.Fetch(ctx, req)
+	if err != nil {
+		return nil, discoveryFailure(ctx, req.CatalogID, err)
+	}
+	return models, nil
 }
 
 // CreateProvider persists a new provider for the account. Providers have no

From 8dcdaeb7993fa652debb48912d569afeaba83446 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Wed, 26 Aug 2026 07:05:44 +0000
Subject: [PATCH 07/16] [management] Let a discovery request name a record and
 a new upstream
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Editing a provider's upstream URL meant retyping its API key. The key is the
one field the API never returns, so there was nothing to retype from, and the
dashboard had to demand it because a provider_id request resolved the upstream
from the stored row — listing the old endpoint while the form showed the new
one.

The request now reads as a partial edit of the record it names, the same way
PUT on a provider already does: an upstream in the body overrides the stored
one, an omitted upstream keeps it. The credential and the catalog entry still
come from the record only, so a caller cannot aim a stored key at a vendor of
their choosing.
---
 .../agentnetwork/credentialcheck_test.go      | 47 +++++++++++++++++++
 .../internals/modules/agentnetwork/manager.go | 17 +++++--
 shared/management/http/api/openapi.yml        |  4 +-
 3 files changed, 62 insertions(+), 6 deletions(-)

diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
index dc0cf133cb6..1d35c585858 100644
--- a/management/internals/modules/agentnetwork/credentialcheck_test.go
+++ b/management/internals/modules/agentnetwork/credentialcheck_test.go
@@ -456,3 +456,50 @@ func TestDiscoverProviderModels_SurfacesTheVendorRefusal(t *testing.T) {
 
 	require.EqualError(t, err, "the provider rejected the credential")
 }
+
+// TestDiscoverProviderModels_ListsAgainstTheUrlOnTheForm covers the edit the
+// operator cannot otherwise make: the upstream has been retyped and the
+// credential has not, because the API never returned it to be retyped. Naming
+// the record supplies the key; the request supplies the URL under test.
+func TestDiscoverProviderModels_ListsAgainstTheUrlOnTheForm(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	// Twice: the create, and the listing, which is gated on Create too.
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+	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
+
+	_, err = f.manager.DiscoverProviderModels(ctx, "account1", "user1", modeldiscovery.Request{
+		CatalogID:   "openai_api",
+		UpstreamURL: "https://gateway.example.com",
+	}, created.ID)
+	require.NoError(t, err)
+
+	asked := f.vendor.only(t)
+	require.Equal(t, "https://gateway.example.com", asked.UpstreamURL, "the typed url must be the one listed against")
+	require.Equal(t, "sk-good", asked.APIKey, "and the stored key must be what lists it")
+}
+
+// TestDiscoverProviderModels_FallsBackToTheStoredUrl keeps the plain refresh
+// working: a request naming only the record still reaches the saved upstream.
+func TestDiscoverProviderModels_FallsBackToTheStoredUrl(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	created, err := f.manager.CreateProvider(ctx, "user1", newCheckedProvider("account1"))
+	require.NoError(t, err)
+	stored := f.vendor.only(t).UpstreamURL
+	f.vendor.requests = nil
+
+	_, err = f.manager.DiscoverProviderModels(ctx, "account1", "user1", modeldiscovery.Request{
+		CatalogID: "openai_api",
+	}, created.ID)
+	require.NoError(t, err)
+
+	require.Equal(t, stored, f.vendor.only(t).UpstreamURL)
+}
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index fe646d55549..9cf5a54e13f 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/management/internals/modules/agentnetwork/manager.go
@@ -204,9 +204,11 @@ func (m *managerImpl) GetProvider(ctx context.Context, accountID, userID, provid
 
 // DiscoverProviderModels asks the vendor which models a credential can reach.
 //
-// recordID, when set, names an existing provider whose stored credential and
-// upstream are used instead of the ones in req — so the dashboard can refresh
-// the list without ever holding the key.
+// recordID, when set, names an existing provider whose stored credential is
+// used instead of the one in req — so the dashboard can refresh the list
+// without ever holding the key. An upstream in req overrides the stored one,
+// which is what lets a form list against a URL the operator has typed but not
+// saved yet, using the credential they cannot retype.
 //
 // Gated on Create rather than Read: this spends the operator's credential
 // against a third party, which is not something a read-only role should be
@@ -227,8 +229,15 @@ func (m *managerImpl) DiscoverProviderModels(ctx context.Context, accountID, use
 		// name a different one would run a provider's credential against
 		// whichever vendor endpoint they picked.
 		req.CatalogID = record.ProviderID
-		req.UpstreamURL = record.UpstreamURL
 		req.APIKey = record.APIKey
+		// The upstream is the one field the caller may override, and only for
+		// entries that serve their listing from it. Those are the operator's
+		// own endpoints, reached with their own credential, so an unsaved URL
+		// is no more reachable here than a saved one — and the SSRF guard
+		// treats both alike.
+		if strings.TrimSpace(req.UpstreamURL) == "" {
+			req.UpstreamURL = record.UpstreamURL
+		}
 	}
 
 	models, err := m.modelDiscovery.Fetch(ctx, req)
diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index c318c8b0494..281bb11ea0d 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -5345,7 +5345,7 @@ components:
         upstream_url:
           type: string
           description: |
-            The upstream being configured. Used to reach vendors that serve their listing from the same host as inference, and to read back the region for those whose host embeds one. Ignored when provider_id is supplied.
+            The upstream being configured. Used to reach vendors that serve their listing from the same host as inference, and to read back the region for those whose host embeds one. Sent alongside provider_id, it overrides the stored upstream, so an edit can be listed against the URL on the form before it is saved.
           example: "https://bedrock-runtime.eu-central-1.amazonaws.com"
         api_key:
           type: string
@@ -5353,7 +5353,7 @@ components:
           example: "sk-..."
         provider_id:
           type: string
-          description: Existing Agent Network provider record whose stored credential and upstream should be used. Lets the form refresh the list without the client holding the key.
+          description: Existing Agent Network provider record to query with. Its stored credential is used, and its upstream unless upstream_url overrides it, so the form can refresh the list without the client holding the key.
           example: "ch8i4ug6lnn4g9hqv7m0"
       required:
         - catalog_provider_id

From b64db1d58711f6b02a7932b85f35379a8dc8c0ec Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Wed, 26 Aug 2026 07:14:37 +0000
Subject: [PATCH 08/16] [management] Check the upstream a Bedrock listing never
 touches

Bedrock lists from the control plane and infers on the runtime host, so a
successful listing says nothing about the URL on the record. An upstream
matching no catalog template left no region to derive, which skipped the check
entirely: a record pointed at a host that does not exist saved clean, and every
request it later served went nowhere.

Entries that declare a listing host of their own now have their configured
upstream resolved on its own account. A host that will not resolve blocks the
save; one that resolves privately does not, since Bedrock behind a proxy is a
supported configuration and stays the unverifiable case it already was.
---
 .../agentnetwork/modeldiscovery/discovery.go  | 51 +++++++++++++++----
 .../modeldiscovery/discovery_test.go          | 47 +++++++++++++++++
 2 files changed, 88 insertions(+), 10 deletions(-)

diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
index 21d4656ec21..c9af4391941 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
@@ -124,6 +124,17 @@ func (c *Client) Fetch(ctx context.Context, req Request) ([]Model, error) {
 		return nil, ErrNoDiscovery
 	}
 
+	// An entry with a listing host of its own answers from somewhere other
+	// than the upstream on the record — Bedrock lists from the control plane
+	// and infers on the runtime host. Reaching the listing therefore proves
+	// nothing about the host requests will actually go to, so that one is
+	// checked separately or not at all.
+	if entry.Discovery.Host != "" {
+		if err := c.checkUpstreamHost(entry, req.UpstreamURL); err != nil {
+			return nil, err
+		}
+	}
+
 	endpoint, err := c.discoveryURL(entry, req)
 	if err != nil {
 		return nil, err
@@ -201,20 +212,40 @@ func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, erro
 	}
 
 	target := &url.URL{Scheme: "https", Host: host, Path: entry.Discovery.Path, RawQuery: entry.Discovery.Query}
-	if err := c.checkPublicHost(target.Hostname()); err != nil {
-		// A host that refuses to resolve fails here, before any request is
-		// built, and it is the commonest way for an upstream to be wrong. It
-		// has to reach the caller as unreachable rather than as an
-		// unclassified fault. ErrPrivateHost is the other outcome and means
-		// something else entirely — not a bad host, one we decline to dial.
-		if errors.Is(err, ErrPrivateHost) {
-			return "", err
-		}
-		return "", &UnreachableError{Provider: entry.Name, Err: err}
+	if err := c.classifyHost(entry, target.Hostname()); err != nil {
+		return "", err
 	}
 	return target.String(), nil
 }
 
+// checkUpstreamHost verifies the host the operator configured, for entries
+// whose listing lives elsewhere and so cannot vouch for it.
+//
+// A name that does not resolve is the record being wrong. One that resolves
+// privately is not: an upstream behind a proxy is a supported configuration,
+// and ErrPrivateHost carries that difference on to the caller, which treats it
+// as unverifiable rather than as a failure.
+func (c *Client) checkUpstreamHost(entry catalog.Provider, upstreamURL string) error {
+	parsed, err := url.Parse(strings.TrimSpace(upstreamURL))
+	if err != nil || parsed.Hostname() == "" {
+		return fmt.Errorf("%w: provider upstream %q is not a usable URL", ErrInvalidRequest, upstreamURL)
+	}
+	return c.classifyHost(entry, parsed.Hostname())
+}
+
+// classifyHost renders a failed host check as the two outcomes the caller
+// distinguishes. A host that refuses to resolve is the commonest way for an
+// upstream to be wrong and has to arrive as unreachable rather than as an
+// unclassified fault. ErrPrivateHost means something else entirely — not a bad
+// host, one we decline to dial.
+func (c *Client) classifyHost(entry catalog.Provider, host string) error {
+	err := c.checkPublicHost(host)
+	if err == nil || errors.Is(err, ErrPrivateHost) {
+		return err
+	}
+	return &UnreachableError{Provider: entry.Name, Err: err}
+}
+
 // RegionFromUpstream recovers the region an operator embedded in the provider
 // upstream, by matching it against the catalog's own host template. Bedrock's
 // template is "bedrock-runtime.<region>.amazonaws.com" and Vertex's is
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
index 47862b573b5..2d3921c3e18 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
@@ -597,3 +597,50 @@ func TestFetch_AProxyInThePathDoesNotSilentlyDisableTheCheck(t *testing.T) {
 type roundTripFunc func(*http.Request) (*http.Response, error)
 
 func (f roundTripFunc) RoundTrip(r *http.Request) (*http.Response, error) { return f(r) }
+
+// TestFetch_TheUpstreamIsCheckedWhenTheListingCannotVouchForIt covers the hole
+// a separate listing host leaves. Bedrock lists from the control plane, so a
+// record whose runtime upstream does not exist reaches a perfectly good
+// listing and saves — the requests it then serves go nowhere.
+//
+// Both halves matter. A runtime host that cannot be resolved is the record
+// being wrong, and blocks. A proxied one resolves and only leaves the region
+// underivable, which stays the unverifiable outcome it already was.
+func TestFetch_TheUpstreamIsCheckedWhenTheListingCannotVouchForIt(t *testing.T) {
+	refusing := &net.Resolver{
+		PreferGo: true,
+		Dial: func(ctx context.Context, network, address string) (net.Conn, error) {
+			return nil, errors.New("resolver unavailable")
+		},
+	}
+	client := &Client{Resolver: refusing}
+
+	_, err := client.Fetch(context.Background(), Request{
+		CatalogID: "bedrock_api",
+		// Matches no catalog template, so nothing here reaches the control
+		// plane the listing comes from: without its own check this upstream
+		// was never contacted at all.
+		UpstreamURL: "https://bedrock.typo.example.invalid",
+		APIKey:      "aws-bearer",
+	})
+
+	require.Error(t, err)
+	var unreachable *UnreachableError
+	require.ErrorAs(t, err, &unreachable, "a runtime host that will not resolve must block the save")
+}
+
+// TestFetch_AListingHostOfItsOwnDoesNotReachThroughTheUpstream keeps the check
+// above from reading the operator's upstream as the place to list from.
+func TestFetch_AListingHostOfItsOwnDoesNotReachThroughTheUpstream(t *testing.T) {
+	cl, tr := newStubClient(http.StatusOK, bedrockListing)
+
+	_, err := cl.Fetch(context.Background(), Request{
+		CatalogID:   "bedrock_api",
+		UpstreamURL: "https://bedrock-runtime.eu-central-1.amazonaws.com",
+		APIKey:      "aws-bearer",
+	})
+	require.NoError(t, err)
+
+	assert.Equal(t, "bedrock.eu-central-1.amazonaws.com", tr.got.URL.Host,
+		"checking the runtime host must not turn it into the listing host")
+}

From 11faa925ee4597413997acb8b7e5eaa121fd890b Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Wed, 26 Aug 2026 14:23:58 +0000
Subject: [PATCH 09/16] [management] Check a vendor change, and bound the check
 by one deadline

Two gaps a review surfaced.

Moving a record from one catalog provider to another changed neither field the
check looked at, so an unchanged credential started being offered to a
different vendor, under a different auth header, with nothing asking whether it
was accepted there.

And each host lookup built its own eight-second budget from a background
context, so a slow resolver could spend one before the request spent another,
and a caller that gave up was still waiting. Bedrock made that three: it now
checks its runtime host as well. One deadline is taken at the top of Fetch and
carried through both lookups and the request.

The create description also had the exemption backwards, reading as though an
upstream the check cannot reach is stored unverified. Unreachable blocks; only
what cannot be checked at all is exempt.
---
 .../agentnetwork/credentialcheck_test.go      | 27 +++++++++++++++++
 .../internals/modules/agentnetwork/manager.go | 12 ++++++--
 .../agentnetwork/modeldiscovery/discovery.go  | 30 +++++++++----------
 .../modeldiscovery/discovery_test.go          |  2 +-
 shared/management/http/api/openapi.yml        |  4 +--
 5 files changed, 54 insertions(+), 21 deletions(-)

diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
index 1d35c585858..1d6400ea8cc 100644
--- a/management/internals/modules/agentnetwork/credentialcheck_test.go
+++ b/management/internals/modules/agentnetwork/credentialcheck_test.go
@@ -503,3 +503,30 @@ func TestDiscoverProviderModels_FallsBackToTheStoredUrl(t *testing.T) {
 
 	require.Equal(t, stored, f.vendor.only(t).UpstreamURL)
 }
+
+// TestUpdateProvider_MovingARecordToAnotherVendorIsChecked covers the edit that
+// changes neither field the vendor judges and still invalidates both. The
+// catalog entry decides which vendor is asked and under which auth header, so
+// the unchanged credential is now being offered somewhere it has never been
+// accepted.
+func TestUpdateProvider_MovingARecordToAnotherVendorIsChecked(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.ProviderID = "anthropic_api"
+	edit.APIKey = ""
+
+	_, err = f.manager.UpdateProvider(ctx, "user1", edit)
+	require.NoError(t, err)
+
+	require.Equal(t, "anthropic_api", f.vendor.only(t).CatalogID,
+		"the new vendor is the one that has to accept the key")
+}
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index 9cf5a54e13f..640ecf06035 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/management/internals/modules/agentnetwork/manager.go
@@ -310,14 +310,20 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide
 		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
+	// Only the 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 catalog entry counts as one of them: it decides which vendor is
+	// asked, under which auth header, so moving a record from one to another
+	// sends an unchanged credential somewhere it has never been accepted.
+	//
 	// 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 provider.UpstreamURL != existing.UpstreamURL ||
+		provider.APIKey != existing.APIKey ||
+		provider.ProviderID != existing.ProviderID {
 		if err := m.checkProviderCredential(ctx, provider); err != nil {
 			return nil, err
 		}
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
index c9af4391941..63103858dd4 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
@@ -124,25 +124,28 @@ func (c *Client) Fetch(ctx context.Context, req Request) ([]Model, error) {
 		return nil, ErrNoDiscovery
 	}
 
+	// One deadline over the whole operation. Both host lookups and the request
+	// itself run under it, so a vendor cannot be slow twice, and a caller that
+	// gives up is not left waiting on a resolver.
+	ctx, cancel := context.WithTimeout(ctx, fetchTimeout)
+	defer cancel()
+
 	// An entry with a listing host of its own answers from somewhere other
 	// than the upstream on the record — Bedrock lists from the control plane
 	// and infers on the runtime host. Reaching the listing therefore proves
 	// nothing about the host requests will actually go to, so that one is
 	// checked separately or not at all.
 	if entry.Discovery.Host != "" {
-		if err := c.checkUpstreamHost(entry, req.UpstreamURL); err != nil {
+		if err := c.checkUpstreamHost(ctx, entry, req.UpstreamURL); err != nil {
 			return nil, err
 		}
 	}
 
-	endpoint, err := c.discoveryURL(entry, req)
+	endpoint, err := c.discoveryURL(ctx, entry, req)
 	if err != nil {
 		return nil, err
 	}
 
-	ctx, cancel := context.WithTimeout(ctx, fetchTimeout)
-	defer cancel()
-
 	httpReq, err := http.NewRequestWithContext(ctx, http.MethodGet, endpoint, nil)
 	if err != nil {
 		return nil, fmt.Errorf("build discovery request: %w", err)
@@ -187,7 +190,7 @@ func (c *Client) Fetch(ctx context.Context, req Request) ([]Model, error) {
 // management holds credentials for every provider, and an upstream pointed at
 // an internal address would turn this endpoint into a probe of the management
 // server's own network.
-func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, error) {
+func (c *Client) discoveryURL(ctx context.Context, entry catalog.Provider, req Request) (string, error) {
 	host := entry.Discovery.Host
 	if host == "" {
 		parsed, err := url.Parse(strings.TrimSpace(req.UpstreamURL))
@@ -212,7 +215,7 @@ func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, erro
 	}
 
 	target := &url.URL{Scheme: "https", Host: host, Path: entry.Discovery.Path, RawQuery: entry.Discovery.Query}
-	if err := c.classifyHost(entry, target.Hostname()); err != nil {
+	if err := c.classifyHost(ctx, entry, target.Hostname()); err != nil {
 		return "", err
 	}
 	return target.String(), nil
@@ -225,12 +228,12 @@ func (c *Client) discoveryURL(entry catalog.Provider, req Request) (string, erro
 // privately is not: an upstream behind a proxy is a supported configuration,
 // and ErrPrivateHost carries that difference on to the caller, which treats it
 // as unverifiable rather than as a failure.
-func (c *Client) checkUpstreamHost(entry catalog.Provider, upstreamURL string) error {
+func (c *Client) checkUpstreamHost(ctx context.Context, entry catalog.Provider, upstreamURL string) error {
 	parsed, err := url.Parse(strings.TrimSpace(upstreamURL))
 	if err != nil || parsed.Hostname() == "" {
 		return fmt.Errorf("%w: provider upstream %q is not a usable URL", ErrInvalidRequest, upstreamURL)
 	}
-	return c.classifyHost(entry, parsed.Hostname())
+	return c.classifyHost(ctx, entry, parsed.Hostname())
 }
 
 // classifyHost renders a failed host check as the two outcomes the caller
@@ -238,8 +241,8 @@ func (c *Client) checkUpstreamHost(entry catalog.Provider, upstreamURL string) e
 // upstream to be wrong and has to arrive as unreachable rather than as an
 // unclassified fault. ErrPrivateHost means something else entirely — not a bad
 // host, one we decline to dial.
-func (c *Client) classifyHost(entry catalog.Provider, host string) error {
-	err := c.checkPublicHost(host)
+func (c *Client) classifyHost(ctx context.Context, entry catalog.Provider, host string) error {
+	err := c.checkPublicHost(ctx, host)
 	if err == nil || errors.Is(err, ErrPrivateHost) {
 		return err
 	}
@@ -283,7 +286,7 @@ func RegionFromUpstream(entry catalog.Provider, upstreamURL string) string {
 
 // checkPublicHost refuses hosts that resolve to an address the management
 // server should never be asked to reach on an operator's behalf.
-func (c *Client) checkPublicHost(host string) error {
+func (c *Client) checkPublicHost(ctx context.Context, host string) error {
 	if c.AllowPrivateHosts {
 		return nil
 	}
@@ -294,9 +297,6 @@ func (c *Client) checkPublicHost(host string) error {
 	if resolver == nil {
 		resolver = net.DefaultResolver
 	}
-	ctx, cancel := context.WithTimeout(context.Background(), fetchTimeout)
-	defer cancel()
-
 	addrs, err := resolver.LookupNetIP(ctx, "ip", host)
 	if err != nil {
 		return fmt.Errorf("resolve discovery host %q: %w", host, err)
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
index 2d3921c3e18..34b6f51cb36 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
@@ -290,7 +290,7 @@ func TestHostGuardRejectsNonPublicAddresses(t *testing.T) {
 
 func TestHostGuardResolvesAndRejectsLocalhost(t *testing.T) {
 	cl := &Client{}
-	err := cl.checkPublicHost("localhost")
+	err := cl.checkPublicHost(context.Background(), "localhost")
 	require.Error(t, err, "a name resolving to loopback must be refused, not just a literal address")
 	assert.Contains(t, err.Error(), "non-public")
 }
diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index 281bb11ea0d..053462e1c9f 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -14146,7 +14146,7 @@ paths:
       description: |
         Connects a new Agent Network AI provider for the account.
 
-        The upstream URL and credential are checked against the vendor before the provider is stored, so a record that cannot reach its vendor is refused rather than saved. Returns 422 with a message naming whichever of the two is at fault. A catalog provider with no listing endpoint, and one whose upstream the check cannot reach from the management service, are stored without being checked.
+        The upstream URL and credential are checked against the vendor before the provider is stored, so a record that cannot reach its vendor is refused rather than saved. Returns 422 with a message naming whichever of the two is at fault — a rejected credential, an upstream that does not resolve or answer, a vendor outage, and a timeout all block the write. Only what cannot be checked at all is exempt and stored unverified: a catalog provider with no listing endpoint, one with no host to derive a listing from, and an upstream resolving to a private address the management service will not dial.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]
@@ -14213,7 +14213,7 @@ paths:
       description: |
         Update an existing Agent Network AI provider.
 
-        When the upstream URL or the API key changes, the pair is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. An update omitting the API key is checked against the stored one. Edits touching neither field are stored without a check.
+        When the upstream URL, the API key or the catalog provider changes, the record is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. An update omitting the API key is checked against the stored one. Edits touching none of the three — a rename, model rows, price edits — are stored without a check, as are the cases the create description lists as unverifiable.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]

From afffc948ebd0c09d7371b809d7cc7b26773b2765 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Wed, 26 Aug 2026 14:34:18 +0000
Subject: [PATCH 10/16] [management] Regenerate the API types after the
 discovery request change
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The generated file carries the schema descriptions as doc comments, so editing
them leaves it behind and the release job's diff check fails. Comments only —
no field or type moved.
---
 shared/management/http/api/types.gen.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go
index db5b2e18e2b..cdd7702ad1a 100644
--- a/shared/management/http/api/types.gen.go
+++ b/shared/management/http/api/types.gen.go
@@ -2202,10 +2202,10 @@ type AgentNetworkModelDiscoveryRequest struct {
 	// CatalogProviderId Catalog provider to query (AgentNetworkCatalogProvider.id). Determines the listing endpoint, the auth header and the response shape.
 	CatalogProviderId string `json:"catalog_provider_id"`
 
-	// ProviderId Existing Agent Network provider record whose stored credential and upstream should be used. Lets the form refresh the list without the client holding the key.
+	// ProviderId Existing Agent Network provider record to query with. Its stored credential is used, and its upstream unless upstream_url overrides it, so the form can refresh the list without the client holding the key.
 	ProviderId *string `json:"provider_id,omitempty"`
 
-	// UpstreamUrl The upstream being configured. Used to reach vendors that serve their listing from the same host as inference, and to read back the region for those whose host embeds one. Ignored when provider_id is supplied.
+	// UpstreamUrl The upstream being configured. Used to reach vendors that serve their listing from the same host as inference, and to read back the region for those whose host embeds one. Sent alongside provider_id, it overrides the stored upstream, so an edit can be listed against the URL on the form before it is saved.
 	UpstreamUrl *string `json:"upstream_url,omitempty"`
 }
 

From 540c551f191691778e25bdcabce3efc3645bc7ad Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Wed, 26 Aug 2026 15:33:16 +0000
Subject: [PATCH 11/16] [management] Make the live rotation test prove what it
 claims
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Two gaps a review found in the live suite.

The rotation test finished by renaming the provider and expecting that to
succeed. A rename touches none of the fields the check looks at, so it is
stored without asking the vendor anything — it would have passed just as well
against a key the rotation had already replaced. It now moves the upstream by a
trailing slash, which reaches the same host but differs as a string, so the
check runs and the stored key is what has to satisfy it.

And the refused-url test only asserted the error. An error is not the same fact
as an absent record, so it now lists the providers and looks for the one that
must not be there — which the refused-key test beside it already did.

The discovery upstream override is logged. It sends the stored credential to a
host the caller named, which the same permission set can already do by pointing
the record there and letting the write check it — but that leaves an activity
event behind, and this would otherwise leave nothing.
---
 .../credential_check_live_test.go             | 19 +++++++++++++++++--
 .../internals/modules/agentnetwork/manager.go | 17 ++++++++++++-----
 2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/e2e/agentnetwork/credential_check_live_test.go b/e2e/agentnetwork/credential_check_live_test.go
index f859cb329dc..f674b578eab 100644
--- a/e2e/agentnetwork/credential_check_live_test.go
+++ b/e2e/agentnetwork/credential_check_live_test.go
@@ -134,6 +134,14 @@ func TestLiveProviderUrlCheck(t *testing.T) {
 	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
@@ -164,8 +172,15 @@ func TestLiveProviderUpdateKeepsTheWorkingKey(t *testing.T) {
 	// 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.
-	renamed := credentialProviderRequest(tc, "e2e-cred-rotate-renamed", "")
-	updated, err := srv.UpdateProvider(ctx, prov.Id, renamed)
+	//
+	// 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)
 }
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index 640ecf06035..69aefc62f2c 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/management/internals/modules/agentnetwork/manager.go
@@ -230,13 +230,20 @@ func (m *managerImpl) DiscoverProviderModels(ctx context.Context, accountID, use
 		// whichever vendor endpoint they picked.
 		req.CatalogID = record.ProviderID
 		req.APIKey = record.APIKey
-		// The upstream is the one field the caller may override, and only for
-		// entries that serve their listing from it. Those are the operator's
-		// own endpoints, reached with their own credential, so an unsaved URL
-		// is no more reachable here than a saved one — and the SSRF guard
-		// treats both alike.
+		// The upstream is the one field the caller may override, so that a URL
+		// typed into the form can be listed against before it is saved.
+		//
+		// It sends the stored credential to a host the caller named, which is
+		// a capability they already have: the same permission set updates the
+		// record's upstream, and that write runs this same check against
+		// whatever it is pointed at. What it would not otherwise be is silent,
+		// since the write leaves an activity event behind — so the override is
+		// recorded here.
 		if strings.TrimSpace(req.UpstreamURL) == "" {
 			req.UpstreamURL = record.UpstreamURL
+		} else if req.UpstreamURL != record.UpstreamURL {
+			log.WithContext(ctx).Infof("agent network provider %s listed against caller-supplied upstream %s by user %s",
+				recordID, req.UpstreamURL, userID)
 		}
 	}
 

From 5720c33be95d6d33311b6cf1dcaa544cf273d9e7 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Thu, 27 Aug 2026 08:15:06 +0000
Subject: [PATCH 12/16] [management] Fix what a second review found in the
 credential check
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Four defects and two pieces of wording, from cubic's pass over the branch.

A provider that asks the proxy to skip TLS verification was checked with a
client that verifies it, so a self-hosted endpoint behind a self-signed
certificate was refused for the one reason its operator had already declared
they accept. Those records now save unchecked, alongside the other cases this
cannot speak for. Sending the credential over a connection management declines
to verify was the other way out, and a worse one.

A key pasted with surrounding whitespace passed its check and then failed every
request: the vendor call trims before building the auth header, the synthesiser
substitutes the stored value verbatim. The key is now stored in the form it
will be sent, so what was checked is what runs.

The proxy-guard test resolved api.openai.com for real before reaching its own
transport, and on a runner with no egress that lookup failed as an
UnreachableError too — so it passed while never exercising the socket guard it
is named for.

A DNS timeout said only that the lookup failed. It now says so as a timeout,
without borrowing the wording of a connection that was never attempted.

The create description promised more than the check delivers: for Bedrock the
runtime host is resolved but never contacted, so a public host that does not
answer is still stored. It says that now, and names the TLS exemption above.
The invalid-upstream message no longer quotes the URL back, which was the one
path in this feature that echoed what the operator typed.

The live suite's single-vendor scope and its dependence on vendor availability
are now stated where the tests are, rather than left to be rediscovered.
---
 .../credential_check_live_test.go             | 16 +++++++
 .../modules/agentnetwork/credentialcheck.go   | 11 +++++
 .../agentnetwork/credentialcheck_test.go      | 46 +++++++++++++++++++
 .../internals/modules/agentnetwork/manager.go | 14 +++++-
 .../agentnetwork/modeldiscovery/discovery.go  |  8 +++-
 .../modeldiscovery/discovery_test.go          |  7 ++-
 .../agentnetwork/modeldiscovery/failure.go    |  7 +++
 shared/management/http/api/openapi.yml        |  6 ++-
 8 files changed, 109 insertions(+), 6 deletions(-)

diff --git a/e2e/agentnetwork/credential_check_live_test.go b/e2e/agentnetwork/credential_check_live_test.go
index f674b578eab..f83a18129b7 100644
--- a/e2e/agentnetwork/credential_check_live_test.go
+++ b/e2e/agentnetwork/credential_check_live_test.go
@@ -67,6 +67,13 @@ func liveCredentialCases() []credentialCase {
 // 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 {
@@ -111,6 +118,12 @@ func TestLiveProviderCredentialCheck(t *testing.T) {
 // 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 {
@@ -147,6 +160,9 @@ func TestLiveProviderUrlCheck(t *testing.T) {
 // 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 {
diff --git a/management/internals/modules/agentnetwork/credentialcheck.go b/management/internals/modules/agentnetwork/credentialcheck.go
index 8306e1ee0c0..a4787d01955 100644
--- a/management/internals/modules/agentnetwork/credentialcheck.go
+++ b/management/internals/modules/agentnetwork/credentialcheck.go
@@ -27,6 +27,17 @@ type ModelLister interface {
 // exercises the path the model picker takes: a URL answering 200 with a login
 // page fails here instead of producing an empty picker later.
 func (m *managerImpl) checkProviderCredential(ctx context.Context, provider *types.Provider) error {
+	// A record that asks the proxy to skip certificate verification is one this
+	// check cannot speak for. Discovery verifies certificates, so a self-hosted
+	// endpoint behind a self-signed one would be refused for a reason the
+	// operator already told us to ignore — a lockout of exactly the setup the
+	// flag exists for. Sending the credential over a connection management
+	// declines to verify is the other way out, and a worse one.
+	if provider.SkipTLSVerification {
+		log.WithContext(ctx).Debugf("agent network provider %s not credential-checked: tls verification is disabled for it", provider.ProviderID)
+		return nil
+	}
+
 	_, err := m.modelDiscovery.Fetch(ctx, modeldiscovery.Request{
 		CatalogID:   provider.ProviderID,
 		UpstreamURL: provider.UpstreamURL,
diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
index 1d6400ea8cc..4e31331a07b 100644
--- a/management/internals/modules/agentnetwork/credentialcheck_test.go
+++ b/management/internals/modules/agentnetwork/credentialcheck_test.go
@@ -530,3 +530,49 @@ func TestUpdateProvider_MovingARecordToAnotherVendorIsChecked(t *testing.T) {
 	require.Equal(t, "anthropic_api", f.vendor.only(t).CatalogID,
 		"the new vendor is the one that has to accept the key")
 }
+
+// TestCreateProvider_ASkipTlsRecordIsNotCheckedAgainstItsCertificate covers the
+// lockout the check would otherwise be: the flag exists for a self-hosted
+// endpoint behind a certificate nothing public can verify, and discovery
+// verifies certificates. Refusing the save would reject the record for the one
+// reason the operator already declared they accept.
+func TestCreateProvider_ASkipTlsRecordIsNotCheckedAgainstItsCertificate(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.vendor.err = &modeldiscovery.UnreachableError{
+		Provider: "OpenAI",
+		Err:      &tls.CertificateVerificationError{},
+	}
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	provider := newCheckedProvider("account1")
+	provider.SkipTLSVerification = true
+
+	created, err := f.manager.CreateProvider(ctx, "user1", provider)
+	require.NoError(t, err, "a record we were told not to verify must still save")
+	require.NotEmpty(t, created.ID)
+	require.Zero(t, f.vendor.calls(), "and the vendor must not be asked at all")
+}
+
+// TestCreateProvider_TheStoredKeyIsTheOneThatWasChecked pins the two halves to
+// one value. The vendor call trims the credential before building its auth
+// header; the synthesiser substitutes the stored one verbatim. A key pasted
+// with surrounding whitespace would otherwise pass its check and then fail
+// every request the provider serves.
+func TestCreateProvider_TheStoredKeyIsTheOneThatWasChecked(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	provider := newCheckedProvider("account1")
+	provider.APIKey = "  sk-good\n"
+
+	created, err := f.manager.CreateProvider(ctx, "user1", provider)
+	require.NoError(t, err)
+
+	require.Equal(t, "sk-good", f.vendor.only(t).APIKey, "the vendor is asked about the trimmed key")
+
+	stored, err := f.store.GetAgentNetworkProviderByID(ctx, store.LockingStrengthNone, "account1", created.ID)
+	require.NoError(t, err)
+	require.Equal(t, "sk-good", stored.APIKey, "and that is the one the proxy will send")
+}
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index 69aefc62f2c..173782e6c4c 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/management/internals/modules/agentnetwork/manager.go
@@ -269,6 +269,11 @@ func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provide
 	if strings.TrimSpace(provider.APIKey) == "" {
 		return nil, status.Errorf(status.InvalidArgument, "api_key is required when creating an agent network provider")
 	}
+	// Stored as it will be sent. The vendor call below trims the key before
+	// building the auth header while the synthesiser substitutes the stored
+	// value verbatim, so a key pasted with surrounding whitespace would pass
+	// its check and then fail every request the provider serves.
+	provider.APIKey = strings.TrimSpace(provider.APIKey)
 
 	// Before anything is persisted: a record whose upstream or credential does
 	// not work is rejected here rather than discovered later as a failed
@@ -311,10 +316,15 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide
 	// Preserve the API key if the caller didn't rotate it. A
 	// whitespace-only value is treated as "not rotated" rather than a
 	// real key, but it must not silently overwrite a valid stored key.
-	if provider.APIKey == "" {
+	switch trimmed := strings.TrimSpace(provider.APIKey); {
+	case provider.APIKey == "":
 		provider.APIKey = existing.APIKey
-	} else if strings.TrimSpace(provider.APIKey) == "" {
+	case trimmed == "":
 		return nil, status.Errorf(status.InvalidArgument, "api_key must be non-blank when rotating an agent network provider")
+	default:
+		// See CreateProvider: the key is stored in the form the proxy will
+		// send, so the check below tests what the provider will actually use.
+		provider.APIKey = trimmed
 	}
 
 	// Only the fields the vendor would judge are worth a round-trip. This same
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
index 63103858dd4..8485e827f90 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery.go
@@ -195,7 +195,11 @@ func (c *Client) discoveryURL(ctx context.Context, entry catalog.Provider, req R
 	if host == "" {
 		parsed, err := url.Parse(strings.TrimSpace(req.UpstreamURL))
 		if err != nil || parsed.Host == "" {
-			return "", fmt.Errorf("%w: provider upstream %q is not a usable URL", ErrInvalidRequest, req.UpstreamURL)
+			// The URL is left out of the message on purpose: it reaches the
+			// operator through an endpoint that does not lowercase it, but the
+			// rest of this feature's copy never echoes what they typed, and one
+			// path that does is the one that ends up quoted in a bug report.
+			return "", fmt.Errorf("%w: the provider upstream is not a usable URL", ErrInvalidRequest)
 		}
 		host = parsed.Host
 	}
@@ -231,7 +235,7 @@ func (c *Client) discoveryURL(ctx context.Context, entry catalog.Provider, req R
 func (c *Client) checkUpstreamHost(ctx context.Context, entry catalog.Provider, upstreamURL string) error {
 	parsed, err := url.Parse(strings.TrimSpace(upstreamURL))
 	if err != nil || parsed.Hostname() == "" {
-		return fmt.Errorf("%w: provider upstream %q is not a usable URL", ErrInvalidRequest, upstreamURL)
+		return fmt.Errorf("%w: the provider upstream is not a usable URL", ErrInvalidRequest)
 	}
 	return c.classifyHost(ctx, entry, parsed.Hostname())
 }
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
index 34b6f51cb36..772f01436e1 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/discovery_test.go
@@ -573,7 +573,12 @@ func TestFetch_AHostThatWillNotResolveIsUnreachable(t *testing.T) {
 func TestFetch_AProxyInThePathDoesNotSilentlyDisableTheCheck(t *testing.T) {
 	// A transport that refuses at the socket exactly as the guard does, with a
 	// loopback address standing in for the proxy the dial went to.
-	client := &Client{HTTPClient: &http.Client{
+	// AllowPrivateHosts short-circuits the resolve-stage check only; the
+	// injected transport below is still what the request goes through. Without
+	// it this test resolves api.openai.com for real, and on a runner with no
+	// egress that lookup fails as an UnreachableError too — so it would pass
+	// while never reaching the socket guard it is named for.
+	client := &Client{AllowPrivateHosts: true, HTTPClient: &http.Client{
 		Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
 			return nil, guardDialAddress("127.0.0.1:38599")
 		}),
diff --git a/management/internals/modules/agentnetwork/modeldiscovery/failure.go b/management/internals/modules/agentnetwork/modeldiscovery/failure.go
index 2b351d75f01..41e4bfb56d5 100644
--- a/management/internals/modules/agentnetwork/modeldiscovery/failure.go
+++ b/management/internals/modules/agentnetwork/modeldiscovery/failure.go
@@ -53,6 +53,13 @@ func (e *UnreachableError) Reason() string {
 		if dns.IsNotFound {
 			return "no such host"
 		}
+		// Named apart from the dial timeout below. A resolver that never
+		// answered and an upstream that never answered send an operator to
+		// different places, and the generic "connection timed out" would
+		// describe a connection that was never attempted.
+		if dns.IsTimeout {
+			return "dns lookup timed out"
+		}
 		return "dns lookup failed"
 	}
 
diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index 053462e1c9f..c37ebf33ae2 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -14146,7 +14146,11 @@ paths:
       description: |
         Connects a new Agent Network AI provider for the account.
 
-        The upstream URL and credential are checked against the vendor before the provider is stored, so a record that cannot reach its vendor is refused rather than saved. Returns 422 with a message naming whichever of the two is at fault — a rejected credential, an upstream that does not resolve or answer, a vendor outage, and a timeout all block the write. Only what cannot be checked at all is exempt and stored unverified: a catalog provider with no listing endpoint, one with no host to derive a listing from, and an upstream resolving to a private address the management service will not dial.
+        The credential is checked against the vendor's model listing before the provider is stored, so a record the vendor will not accept is refused rather than saved. Returns 422 naming what is at fault — a rejected credential, a listing endpoint that does not resolve or answer, a vendor outage, and a timeout all block the write.
+
+        How much of the upstream URL that covers depends on the provider. Where the listing is served from the upstream itself, reaching it proves the URL. Where the catalog entry has a listing host of its own — Bedrock, whose listing comes from the control plane — the configured runtime host is resolved on its own account but never contacted, so a public host that does not answer is still stored.
+
+        Only what cannot be checked at all is exempt and stored unverified: a catalog provider with no listing endpoint, one with no host to derive a listing from, an upstream resolving to a private address the management service will not dial, and a provider configured to skip TLS verification.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]

From 8a716f12945ad47879d7ced3c5aa38b05fdfbb15 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Thu, 27 Aug 2026 08:33:57 +0000
Subject: [PATCH 13/16] [management] Check a record when TLS verification is
 switched back on
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The skip-TLS exemption added a hole of its own. Such a record is stored without
being checked at all, and the re-check on update watched only the upstream, the
key and the catalog entry — so turning verification back on left a provider
that had never been checked now running as if it had. That transition is the
first moment the record can be checked, and it now is.

An update that preserves the stored key trims it on the way through, so a
record saved before keys were normalised is repaired by the next edit rather
than carrying whitespace the proxy still sends. That makes the comparison see a
change, which is right: the key has never been tested in the form it is about
to be sent in.

The create description also claimed more of the upstream than the check reads.
Only the host is used — the listing goes to the catalog's own path over HTTPS —
so a configured scheme or path is neither used nor validated there.
---
 .../agentnetwork/credentialcheck_test.go      | 27 +++++++++++++++++++
 .../internals/modules/agentnetwork/manager.go | 15 +++++++++--
 shared/management/http/api/openapi.yml        |  4 +--
 3 files changed, 42 insertions(+), 4 deletions(-)

diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go
index 4e31331a07b..8bc8f0b58d8 100644
--- a/management/internals/modules/agentnetwork/credentialcheck_test.go
+++ b/management/internals/modules/agentnetwork/credentialcheck_test.go
@@ -576,3 +576,30 @@ func TestCreateProvider_TheStoredKeyIsTheOneThatWasChecked(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "sk-good", stored.APIKey, "and that is the one the proxy will send")
 }
+
+// TestUpdateProvider_TurningTlsVerificationBackOnChecksTheRecord covers the
+// hole the skip-TLS exemption opens on its own. Such a record is stored without
+// ever being checked, so the moment verification is switched back on is the
+// first moment it can be checked at all — and none of the three fields the
+// re-check usually watches has to move for that to happen.
+func TestUpdateProvider_TurningTlsVerificationBackOnChecksTheRecord(t *testing.T) {
+	ctx := context.Background()
+	f := newBootstrapFixture(t)
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Create, true)
+
+	unchecked := newCheckedProvider("account1")
+	unchecked.SkipTLSVerification = true
+	created, err := f.manager.CreateProvider(ctx, "user1", unchecked)
+	require.NoError(t, err)
+	require.Zero(t, f.vendor.calls(), "the create was exempt")
+
+	f.expectPermission("account1", "user1", modules.AgentNetworkProviders, operations.Update, true)
+	edit := newCheckedProvider("account1")
+	edit.ID = created.ID
+	edit.APIKey = ""
+	edit.SkipTLSVerification = false
+
+	_, err = f.manager.UpdateProvider(ctx, "user1", edit)
+	require.NoError(t, err)
+	require.Equal(t, 1, f.vendor.calls(), "switching verification on must check what was never checked")
+}
diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go
index 173782e6c4c..c3d396b6666 100644
--- a/management/internals/modules/agentnetwork/manager.go
+++ b/management/internals/modules/agentnetwork/manager.go
@@ -318,7 +318,12 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide
 	// real key, but it must not silently overwrite a valid stored key.
 	switch trimmed := strings.TrimSpace(provider.APIKey); {
 	case provider.APIKey == "":
-		provider.APIKey = existing.APIKey
+		// Trimmed on the way through: a record stored before keys were
+		// normalised carries whitespace the proxy still sends, and an edit
+		// that preserves the key is the occasion to repair it. Doing so makes
+		// the comparison below see a change, which is correct — that key has
+		// never been tested in the form it is about to be sent in.
+		provider.APIKey = strings.TrimSpace(existing.APIKey)
 	case trimmed == "":
 		return nil, status.Errorf(status.InvalidArgument, "api_key must be non-blank when rotating an agent network provider")
 	default:
@@ -338,9 +343,15 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide
 	// 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.
+	//
+	// Turning TLS verification back on is the fourth: the record was stored
+	// unchecked precisely because that flag was set, so this is the first
+	// moment it can be checked at all, and nothing else about it need change
+	// for that to be true.
 	if provider.UpstreamURL != existing.UpstreamURL ||
 		provider.APIKey != existing.APIKey ||
-		provider.ProviderID != existing.ProviderID {
+		provider.ProviderID != existing.ProviderID ||
+		(existing.SkipTLSVerification && !provider.SkipTLSVerification) {
 		if err := m.checkProviderCredential(ctx, provider); err != nil {
 			return nil, err
 		}
diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index c37ebf33ae2..f3807e15e0d 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -14146,9 +14146,9 @@ paths:
       description: |
         Connects a new Agent Network AI provider for the account.
 
-        The credential is checked against the vendor's model listing before the provider is stored, so a record the vendor will not accept is refused rather than saved. Returns 422 naming what is at fault — a rejected credential, a listing endpoint that does not resolve or answer, a vendor outage, and a timeout all block the write.
+        The credential is checked against the vendor's model listing before the provider is stored, so a record the vendor will not accept is refused rather than saved. A rejected credential, a listing endpoint that does not resolve or answer, a vendor outage, and a timeout all block the write and return 422.
 
-        How much of the upstream URL that covers depends on the provider. Where the listing is served from the upstream itself, reaching it proves the URL. Where the catalog entry has a listing host of its own — Bedrock, whose listing comes from the control plane — the configured runtime host is resolved on its own account but never contacted, so a public host that does not answer is still stored.
+        What that proves about the upstream URL is narrower than the URL itself. Only its host is used: the listing is requested over HTTPS at the path the catalog entry declares, so a configured scheme or path is neither used nor validated here. Where the catalog entry has a listing host of its own — Bedrock, whose listing comes from the control plane — even the host is only resolved, never contacted, so a public host that does not answer is still stored.
 
         Only what cannot be checked at all is exempt and stored unverified: a catalog provider with no listing endpoint, one with no host to derive a listing from, an upstream resolving to a private address the management service will not dial, and a provider configured to skip TLS verification.
       tags: [ Agent Network ]

From e85e18e5b961f5a3658ba9d6330af149a47ca4c9 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Thu, 27 Aug 2026 08:46:24 +0000
Subject: [PATCH 14/16] [management] Document the fourth trigger for the update
 credential check

---
 shared/management/http/api/openapi.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index f3807e15e0d..38b086abbc1 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -14217,7 +14217,7 @@ paths:
       description: |
         Update an existing Agent Network AI provider.
 
-        When the upstream URL, the API key or the catalog provider changes, the record is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. An update omitting the API key is checked against the stored one. Edits touching none of the three — a rename, model rows, price edits — are stored without a check, as are the cases the create description lists as unverifiable.
+        When the upstream URL, the API key or the catalog provider changes, the record is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. Switching TLS verification back on is the fourth trigger: a provider exempt from the check was stored unverified, so the edit that ends the exemption is the first opportunity to check it. An update omitting the API key is checked against the stored one. Edits touching none of the four — a rename, model rows, price edits — are stored without a check, as are the cases the create description lists as unverifiable.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]

From 7ca9be2f2f139c80e64ea60440731b576a343afd Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Thu, 27 Aug 2026 08:48:13 +0000
Subject: [PATCH 15/16] [management] Refuse a blank api_key on a provider
 update instead of ignoring it

---
 .../handlers/providers_handler.go             |  8 +++++
 .../handlers/providers_handler_test.go        | 33 +++++++++++++++++++
 2 files changed, 41 insertions(+)

diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler.go b/management/internals/modules/agentnetwork/handlers/providers_handler.go
index 645d1da619e..64b0e4e0ae2 100644
--- a/management/internals/modules/agentnetwork/handlers/providers_handler.go
+++ b/management/internals/modules/agentnetwork/handlers/providers_handler.go
@@ -339,6 +339,14 @@ func validate(req *api.AgentNetworkProviderRequest, requireAPIKey bool) error {
 	if requireAPIKey && (req.ApiKey == nil || strings.TrimSpace(*req.ApiKey) == "") {
 		return status.Errorf(status.InvalidArgument, "api_key is required")
 	}
+	// An update omits api_key to keep the stored credential. A key that is
+	// present but blank is not that: Provider.FromAPIRequest drops it exactly
+	// as if it were absent, so a rotation the operator believes they performed
+	// would answer 200 having changed nothing. Refuse it here, where the
+	// request still carries the difference between absent and blank.
+	if req.ApiKey != nil && strings.TrimSpace(*req.ApiKey) == "" {
+		return status.Errorf(status.InvalidArgument, "api_key must be omitted to keep the stored credential rather than sent blank")
+	}
 	if req.Models != nil {
 		for i, m := range *req.Models {
 			if err := validateModel(i, m); err != nil {
diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler_test.go b/management/internals/modules/agentnetwork/handlers/providers_handler_test.go
index c32835a562f..033de3b8aa6 100644
--- a/management/internals/modules/agentnetwork/handlers/providers_handler_test.go
+++ b/management/internals/modules/agentnetwork/handlers/providers_handler_test.go
@@ -54,6 +54,39 @@ func TestValidate_ModelRates(t *testing.T) {
 	}
 }
 
+// TestValidate_ABlankApiKeyIsNotTheSameAsAnOmittedOne covers the one shape the
+// manager's own guard cannot see. Provider.FromAPIRequest assigns the key only
+// when it trims to something, so a request carrying "   " arrives at
+// UpdateProvider indistinguishable from one that omitted it — the stored
+// credential is kept and the write answers 200, telling an operator who thinks
+// they just rotated a key that it worked.
+//
+// The request still knows the difference, so the refusal belongs here.
+func TestValidate_ABlankApiKeyIsNotTheSameAsAnOmittedOne(t *testing.T) {
+	req := func(key *string) *api.AgentNetworkProviderRequest {
+		return &api.AgentNetworkProviderRequest{
+			ProviderId:  "openai_api",
+			Name:        "OpenAI",
+			UpstreamUrl: "https://api.openai.com",
+			ApiKey:      key,
+		}
+	}
+
+	blank := "   "
+	err := validate(req(&blank), false)
+	require.Error(t, err, "a blank api_key on update must not be read as 'keep what is stored'")
+	assert.Contains(t, err.Error(), "api_key")
+
+	require.NoError(t, validate(req(nil), false), "an omitted api_key is how an update keeps the stored credential")
+
+	// Create already refuses this, and keeps its own message: a caller who sent
+	// no usable key is told the field is required rather than being told how to
+	// preserve a credential that does not exist yet.
+	err = validate(req(&blank), true)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "api_key is required")
+}
+
 // TestProviderHandler_UpdateReplacesFullState pins the update contract shared
 // with the other PUT endpoints: the request replaces the provider's mutable
 // state, so optional fields absent from the JSON land as their zero values.

From 564df341128521e53ca3fb27569e1369d6bb0778 Mon Sep 17 00:00:00 2001
From: mlsmaycon <mlsmaycon@gmail.com>
Date: Thu, 27 Aug 2026 09:03:25 +0000
Subject: [PATCH 16/16] [management] Scope the omitted-key sentence to updates
 that trigger a check

---
 shared/management/http/api/openapi.yml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml
index 38b086abbc1..1ad9c672a81 100644
--- a/shared/management/http/api/openapi.yml
+++ b/shared/management/http/api/openapi.yml
@@ -14217,7 +14217,7 @@ paths:
       description: |
         Update an existing Agent Network AI provider.
 
-        When the upstream URL, the API key or the catalog provider changes, the record is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. Switching TLS verification back on is the fourth trigger: a provider exempt from the check was stored unverified, so the edit that ends the exemption is the first opportunity to check it. An update omitting the API key is checked against the stored one. Edits touching none of the four — a rename, model rows, price edits — are stored without a check, as are the cases the create description lists as unverifiable.
+        When the upstream URL, the API key or the catalog provider changes, the record is checked against the vendor before the change is stored, and a refusal returns 422 without replacing what was there. Switching TLS verification back on is the fourth trigger: a provider exempt from the check was stored unverified, so the edit that ends the exemption is the first opportunity to check it. Where one of the four does fire, an update that omits the API key is checked against the stored one. Edits touching none of them — a rename, model rows, price edits — are stored without a check, as are the cases the create description lists as unverifiable.
       tags: [ Agent Network ]
       security:
         - BearerAuth: [ ]