diff --git a/e2e/agentnetwork/credential_check_live_test.go b/e2e/agentnetwork/credential_check_live_test.go index f859cb329..f674b578e 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 640ecf060..69aefc62f 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) } }