diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go index dc0cf133c..1d35c5858 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 fe646d555..9cf5a54e1 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 c318c8b04..281bb11ea 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