[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.

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")
+}

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
 		}

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)

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")
 }

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: [ ]