diff --git a/management/internals/modules/agentnetwork/credentialcheck_test.go b/management/internals/modules/agentnetwork/credentialcheck_test.go index d167a85a6..3ea85ba81 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 05024cde9..c32835a56 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 b3f96c0a4..21d4656ec 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 9b39f387e..47862b573 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 95f9c35dc..b046581e8 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 d4efb1607..d438ffbdd 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"}},