diff --git a/management/internals/modules/agentnetwork/catalog/catalog.go b/management/internals/modules/agentnetwork/catalog/catalog.go index fe47a33a2..951bb3a6d 100644 --- a/management/internals/modules/agentnetwork/catalog/catalog.go +++ b/management/internals/modules/agentnetwork/catalog/catalog.go @@ -40,6 +40,18 @@ const ( KindCustom ProviderKind = "custom" ) +// AuthMode describes whether a catalog provider accepts an upstream +// credential. The zero value intentionally resolves to AuthModeRequired so +// existing and newly-added catalog entries fail closed unless they explicitly +// opt into a less restrictive mode. +type AuthMode string + +const ( + AuthModeRequired AuthMode = "required" + AuthModeOptional AuthMode = "optional" + AuthModeNone AuthMode = "none" +) + // Provider is the in-memory representation of a catalog provider. type Provider struct { ID string @@ -48,6 +60,9 @@ type Provider struct { DefaultHost string // Kind groups this entry for UI presentation; see ProviderKind. Kind ProviderKind + // AuthMode declares whether the provider requires, optionally accepts, or + // does not support an upstream API key. Empty defaults to required. + AuthMode AuthMode // AuthHeaderName is the HTTP header the provider's API expects // the credential under (e.g. "Authorization" for OpenAI, // "x-api-key" for Anthropic). Combined with AuthHeaderTemplate @@ -704,9 +719,11 @@ var providers = []Provider{ { // Ollama exposes an OpenAI-compatible /v1 API. Like vLLM, it gets a // dedicated catalog id for provider-specific setup guidance while - // retaining the generic custom-provider routing and auth behavior. + // retaining generic custom-provider routing. Authentication is optional + // because local Ollama is authless but protected front ends may use it. ID: "ollama", Kind: KindCustom, + AuthMode: AuthModeOptional, Name: "Ollama", Description: "Self-hosted Ollama (OpenAI-compatible)", DefaultHost: "", @@ -753,6 +770,15 @@ func IsKnown(id string) bool { return ok } +// EffectiveAuthMode returns the provider's configured authentication mode. +// Treating the zero value as required keeps older catalog entries fail closed. +func (p Provider) EffectiveAuthMode() AuthMode { + if p.AuthMode == "" { + return AuthModeRequired + } + return p.AuthMode +} + // IsVertexPathStyle reports whether a provider uses the Google Vertex AI // request shape — the model is carried in the URL path // (/v1/projects/{p}/locations/{r}/publishers/{pub}/models/{model}:{action}) @@ -794,6 +820,7 @@ func (p Provider) ToAPIResponse() api.AgentNetworkCatalogProvider { Description: p.Description, DefaultHost: p.DefaultHost, Kind: kind, + AuthMode: api.AgentNetworkCatalogProviderAuthMode(p.EffectiveAuthMode()), AuthHeaderTemplate: p.AuthHeaderTemplate, DefaultContentType: p.DefaultContentType, BrandColor: p.BrandColor, diff --git a/management/internals/modules/agentnetwork/catalog/catalog_test.go b/management/internals/modules/agentnetwork/catalog/catalog_test.go index 96b950833..c1020e05b 100644 --- a/management/internals/modules/agentnetwork/catalog/catalog_test.go +++ b/management/internals/modules/agentnetwork/catalog/catalog_test.go @@ -14,18 +14,41 @@ func TestOllamaCatalogEntry(t *testing.T) { require.True(t, ok, "Ollama must be available as a dedicated catalog provider") assert.Equal(t, KindCustom, entry.Kind) + assert.Equal(t, AuthModeOptional, entry.EffectiveAuthMode()) assert.Equal(t, "Ollama", entry.Name) assert.Equal(t, "Self-hosted Ollama (OpenAI-compatible)", entry.Description) assert.Empty(t, entry.DefaultHost, "the dashboard owns Ollama's scheme-aware HTTP placeholder") assert.Equal(t, "Authorization", entry.AuthHeaderName) assert.Equal(t, "Bearer ${API_KEY}", entry.AuthHeaderTemplate) assert.Equal(t, "application/json", entry.DefaultContentType) - assert.Empty(t, entry.ParserID, "phase one must preserve the untagged vLLM/custom routing behavior") + assert.Empty(t, entry.ParserID, "Ollama preserves the untagged vLLM/custom routing behavior") assert.Empty(t, entry.Models, "Ollama models are installed dynamically on the configured endpoint") wire := entry.ToAPIResponse() assert.Equal(t, "ollama", wire.Id) assert.Equal(t, api.AgentNetworkCatalogProviderKindCustom, wire.Kind) + assert.Equal(t, api.AgentNetworkCatalogProviderAuthModeOptional, wire.AuthMode) assert.NotNil(t, wire.Models) assert.Empty(t, wire.Models) } + +func TestCatalogAuthenticationModes(t *testing.T) { + openAI, ok := Lookup("openai_api") + require.True(t, ok) + assert.Empty(t, openAI.AuthMode, "existing entries use the fail-closed default") + assert.Equal(t, AuthModeRequired, openAI.EffectiveAuthMode()) + assert.Equal(t, api.AgentNetworkCatalogProviderAuthModeRequired, openAI.ToAPIResponse().AuthMode) + + for _, entry := range All() { + switch entry.EffectiveAuthMode() { + case AuthModeRequired, AuthModeOptional: + assert.NotEmpty(t, entry.AuthHeaderName, "%s must declare an auth header name", entry.ID) + assert.NotEmpty(t, entry.AuthHeaderTemplate, "%s must declare an auth header template", entry.ID) + case AuthModeNone: + assert.Empty(t, entry.AuthHeaderName, "%s must not declare an auth header name", entry.ID) + assert.Empty(t, entry.AuthHeaderTemplate, "%s must not declare an auth header template", entry.ID) + default: + t.Errorf("%s has invalid auth mode %q", entry.ID, entry.AuthMode) + } + } +} diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler.go b/management/internals/modules/agentnetwork/handlers/providers_handler.go index 13da137d5..f4ee45679 100644 --- a/management/internals/modules/agentnetwork/handlers/providers_handler.go +++ b/management/internals/modules/agentnetwork/handlers/providers_handler.go @@ -193,11 +193,12 @@ func (h *handler) deleteProvider(w http.ResponseWriter, r *http.Request) { util.WriteJSONObject(r.Context(), w, util.EmptyObject{}) } -func validate(req *api.AgentNetworkProviderRequest, requireAPIKey bool) error { +func validate(req *api.AgentNetworkProviderRequest, creating bool) error { if strings.TrimSpace(req.ProviderId) == "" { return status.Errorf(status.InvalidArgument, "provider_id is required") } - if !catalog.IsKnown(req.ProviderId) { + entry, ok := catalog.Lookup(req.ProviderId) + if !ok { return status.Errorf(status.InvalidArgument, "provider_id %q is not a known catalog provider", req.ProviderId) } if strings.TrimSpace(req.Name) == "" { @@ -210,8 +211,27 @@ func validate(req *api.AgentNetworkProviderRequest, requireAPIKey bool) error { if err != nil || u.Host == "" || (u.Scheme != "http" && u.Scheme != "https") { return status.Errorf(status.InvalidArgument, "upstream_url must be a full http(s) URL") } - if requireAPIKey && (req.ApiKey == nil || strings.TrimSpace(*req.ApiKey) == "") { - return status.Errorf(status.InvalidArgument, "api_key is required") + return validateProviderAPIKey(entry, req.ApiKey, creating) +} + +func validateProviderAPIKey(entry catalog.Provider, apiKey *string, creating bool) error { + apiKeyBlank := apiKey == nil || strings.TrimSpace(*apiKey) == "" + switch entry.EffectiveAuthMode() { + case catalog.AuthModeRequired: + // Updates may omit the key to preserve it, but an explicitly empty + // value cannot clear a credential required by the selected provider. + if (creating || apiKey != nil) && apiKeyBlank { + return status.Errorf(status.InvalidArgument, "api_key is required for provider_id %q", entry.ID) + } + case catalog.AuthModeOptional: + // Both omitted and explicitly empty values are valid. The manager + // distinguishes preserve from clear during update. + case catalog.AuthModeNone: + if !apiKeyBlank { + return status.Errorf(status.InvalidArgument, "api_key is not supported for provider_id %q", entry.ID) + } + default: + return status.Errorf(status.InvalidArgument, "provider_id %q has an invalid authentication mode", entry.ID) } return nil } diff --git a/management/internals/modules/agentnetwork/handlers/providers_validation_test.go b/management/internals/modules/agentnetwork/handlers/providers_validation_test.go new file mode 100644 index 000000000..a6cdd027a --- /dev/null +++ b/management/internals/modules/agentnetwork/handlers/providers_validation_test.go @@ -0,0 +1,47 @@ +package handlers + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/catalog" +) + +func TestValidateProviderAPIKey(t *testing.T) { + key := "secret" + empty := "" + + tests := []struct { + name string + mode catalog.AuthMode + apiKey *string + creating bool + wantErr string + }{ + {name: "required create with key", mode: catalog.AuthModeRequired, apiKey: &key, creating: true}, + {name: "required create omitted", mode: catalog.AuthModeRequired, creating: true, wantErr: "required"}, + {name: "required update omitted preserves", mode: catalog.AuthModeRequired}, + {name: "required update explicit empty", mode: catalog.AuthModeRequired, apiKey: &empty, wantErr: "required"}, + {name: "optional create omitted", mode: catalog.AuthModeOptional, creating: true}, + {name: "optional update explicit empty", mode: catalog.AuthModeOptional, apiKey: &empty}, + {name: "optional with key", mode: catalog.AuthModeOptional, apiKey: &key, creating: true}, + {name: "none omitted", mode: catalog.AuthModeNone, creating: true}, + {name: "none explicit empty", mode: catalog.AuthModeNone, apiKey: &empty}, + {name: "none with key", mode: catalog.AuthModeNone, apiKey: &key, wantErr: "not supported"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + entry := catalog.Provider{ID: "test-provider", AuthMode: tt.mode} + err := validateProviderAPIKey(entry, tt.apiKey, tt.creating) + if tt.wantErr == "" { + require.NoError(t, err) + return + } + require.Error(t, err) + assert.Contains(t, err.Error(), tt.wantErr) + }) + } +} diff --git a/management/internals/modules/agentnetwork/manager.go b/management/internals/modules/agentnetwork/manager.go index d88e0d77c..2bcad7b8c 100644 --- a/management/internals/modules/agentnetwork/manager.go +++ b/management/internals/modules/agentnetwork/manager.go @@ -12,6 +12,7 @@ import ( log "github.com/sirupsen/logrus" + "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/catalog" "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/labelgen" "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types" "github.com/netbirdio/netbird/management/internals/modules/reverseproxy/proxy" @@ -174,11 +175,8 @@ func (m *managerImpl) CreateProvider(ctx context.Context, userID string, provide return nil, err } - // An empty api_key would silently produce a synthesised service - // that 401s on every upstream request. Surface the misconfiguration - // at create time instead. - if strings.TrimSpace(provider.APIKey) == "" { - return nil, status.Errorf(status.InvalidArgument, "api_key is required when creating an agent network provider") + if err := prepareProviderAPIKey(provider, nil); err != nil { + return nil, err } if provider.ID == "" { @@ -222,13 +220,8 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide return nil, fmt.Errorf("failed to get agent network provider: %w", err) } - // 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 == "" { - provider.APIKey = existing.APIKey - } else if strings.TrimSpace(provider.APIKey) == "" { - return nil, status.Errorf(status.InvalidArgument, "api_key must be non-blank when rotating an agent network provider") + if err := prepareProviderAPIKey(provider, existing); err != nil { + return nil, err } // Always preserve the session keypair across updates so existing // session cookies stay valid. The keys are server-managed and @@ -251,6 +244,52 @@ func (m *managerImpl) UpdateProvider(ctx context.Context, userID string, provide return provider, nil } +// prepareProviderAPIKey applies catalog-owned authentication semantics before +// a provider is persisted. existing is nil on create. On update, omission +// preserves a key only when the provider type is unchanged; switching types +// never carries an old provider's secret into the new upstream. +func prepareProviderAPIKey(provider, existing *types.Provider) error { + entry, ok := catalog.Lookup(provider.ProviderID) + if !ok { + return status.Errorf(status.InvalidArgument, "provider_id %q is not a known catalog provider", provider.ProviderID) + } + return prepareProviderAPIKeyForEntry(provider, existing, entry) +} + +func prepareProviderAPIKeyForEntry(provider, existing *types.Provider, entry catalog.Provider) error { + keyProvided := provider.APIKeyProvided || provider.APIKey != "" + providerChanged := existing != nil && provider.ProviderID != existing.ProviderID + if existing != nil && !keyProvided && !providerChanged && entry.EffectiveAuthMode() != catalog.AuthModeNone { + provider.APIKey = existing.APIKey + } + // Normalize after a preserved value is restored so a legacy + // whitespace-only credential cannot pass manager validation and then fail + // later during synthesis. + if strings.TrimSpace(provider.APIKey) == "" { + provider.APIKey = "" + } + + switch entry.EffectiveAuthMode() { + case catalog.AuthModeRequired: + if provider.APIKey == "" { + return status.Errorf(status.InvalidArgument, "api_key is required for provider_id %q", provider.ProviderID) + } + case catalog.AuthModeOptional: + // Empty is a valid credential state. + case catalog.AuthModeNone: + if keyProvided && provider.APIKey != "" { + return status.Errorf(status.InvalidArgument, "api_key is not supported for provider_id %q", provider.ProviderID) + } + // Clear any stale credential already stored for a provider whose + // catalog semantics changed to no authentication. + provider.APIKey = "" + default: + return status.Errorf(status.InvalidArgument, "provider_id %q has an invalid authentication mode", provider.ProviderID) + } + + return nil +} + func (m *managerImpl) DeleteProvider(ctx context.Context, accountID, userID, providerID string) error { if err := m.requirePermission(ctx, accountID, userID, operations.Delete); err != nil { return err diff --git a/management/internals/modules/agentnetwork/provider_auth_test.go b/management/internals/modules/agentnetwork/provider_auth_test.go new file mode 100644 index 000000000..6e22f0dc3 --- /dev/null +++ b/management/internals/modules/agentnetwork/provider_auth_test.go @@ -0,0 +1,107 @@ +package agentnetwork + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/catalog" + "github.com/netbirdio/netbird/management/internals/modules/agentnetwork/types" +) + +func TestPrepareProviderAPIKey(t *testing.T) { + t.Run("required create rejects an empty key", func(t *testing.T) { + provider := &types.Provider{ProviderID: "openai_api"} + err := prepareProviderAPIKey(provider, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "api_key is required") + }) + + t.Run("optional create accepts an empty key", func(t *testing.T) { + provider := &types.Provider{ProviderID: "ollama"} + require.NoError(t, prepareProviderAPIKey(provider, nil)) + assert.Empty(t, provider.APIKey) + }) + + t.Run("optional update omission preserves the existing key", func(t *testing.T) { + existing := &types.Provider{ProviderID: "ollama", APIKey: "existing"} + provider := &types.Provider{ProviderID: "ollama"} + require.NoError(t, prepareProviderAPIKey(provider, existing)) + assert.Equal(t, "existing", provider.APIKey) + }) + + t.Run("optional update explicit empty clears the existing key", func(t *testing.T) { + existing := &types.Provider{ProviderID: "ollama", APIKey: "existing"} + provider := &types.Provider{ProviderID: "ollama", APIKeyProvided: true} + require.NoError(t, prepareProviderAPIKey(provider, existing)) + assert.Empty(t, provider.APIKey) + }) + + t.Run("changing to optional without a key does not carry the old secret", func(t *testing.T) { + existing := &types.Provider{ProviderID: "openai_api", APIKey: "sk-old-provider"} + provider := &types.Provider{ProviderID: "ollama"} + require.NoError(t, prepareProviderAPIKey(provider, existing)) + assert.Empty(t, provider.APIKey) + }) + + t.Run("changing to required without a key fails", func(t *testing.T) { + existing := &types.Provider{ProviderID: "ollama", APIKey: "old-optional-key"} + provider := &types.Provider{ProviderID: "openai_api"} + err := prepareProviderAPIKey(provider, existing) + require.Error(t, err) + assert.Contains(t, err.Error(), "api_key is required") + assert.Empty(t, provider.APIKey) + }) + + t.Run("required update omission preserves the existing key", func(t *testing.T) { + existing := &types.Provider{ProviderID: "openai_api", APIKey: "sk-existing"} + provider := &types.Provider{ProviderID: "openai_api"} + require.NoError(t, prepareProviderAPIKey(provider, existing)) + assert.Equal(t, "sk-existing", provider.APIKey) + }) + + t.Run("required update rejects a preserved whitespace-only key", func(t *testing.T) { + existing := &types.Provider{ProviderID: "openai_api", APIKey: " "} + provider := &types.Provider{ProviderID: "openai_api"} + err := prepareProviderAPIKey(provider, existing) + require.Error(t, err) + assert.Contains(t, err.Error(), "api_key is required") + assert.Empty(t, provider.APIKey) + }) + + t.Run("required update explicit empty is rejected", func(t *testing.T) { + existing := &types.Provider{ProviderID: "openai_api", APIKey: "sk-existing"} + provider := &types.Provider{ProviderID: "openai_api", APIKeyProvided: true} + err := prepareProviderAPIKey(provider, existing) + require.Error(t, err) + assert.Contains(t, err.Error(), "api_key is required") + }) + + t.Run("unknown catalog provider is rejected", func(t *testing.T) { + provider := &types.Provider{ProviderID: "unknown", APIKey: "secret"} + err := prepareProviderAPIKey(provider, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "not a known catalog provider") + }) + + t.Run("none mode clears a stale key", func(t *testing.T) { + entry := catalog.Provider{ID: "authless", AuthMode: catalog.AuthModeNone} + existing := &types.Provider{ProviderID: "authless", APIKey: "stale-secret"} + provider := &types.Provider{ProviderID: "authless"} + require.NoError(t, prepareProviderAPIKeyForEntry(provider, existing, entry)) + assert.Empty(t, provider.APIKey) + }) + + t.Run("none mode rejects a supplied key", func(t *testing.T) { + entry := catalog.Provider{ID: "authless", AuthMode: catalog.AuthModeNone} + provider := &types.Provider{ + ProviderID: "authless", + APIKey: "unexpected-secret", + APIKeyProvided: true, + } + err := prepareProviderAPIKeyForEntry(provider, nil, entry) + require.Error(t, err) + assert.Contains(t, err.Error(), "not supported") + }) +} diff --git a/management/internals/modules/agentnetwork/synthesizer.go b/management/internals/modules/agentnetwork/synthesizer.go index 95fe91773..98165a66f 100644 --- a/management/internals/modules/agentnetwork/synthesizer.go +++ b/management/internals/modules/agentnetwork/synthesizer.go @@ -906,23 +906,33 @@ const ( noopUpstreamPort = uint16(443) ) -// providerAuthHeader builds the upstream auth header pair for a -// provider from its catalog entry. The catalog declares which header -// name and template a provider's API expects; the synthesiser -// substitutes the provider's decrypted API key into the template and -// returns the (name, value) pair the router middleware injects after -// stripping the inbound vendor auth headers. +// providerAuthHeader builds the upstream auth header pair for a provider from +// its catalog entry. Optional providers with no key and providers using no +// authentication return an empty pair. Otherwise, the synthesiser substitutes +// the decrypted API key into the catalog template and returns the pair the +// router injects after stripping inbound vendor credentials. func providerAuthHeader(p *types.Provider) (name, value, gcpSAKeyB64 string, err error) { entry, ok := catalog.Lookup(p.ProviderID) if !ok { return "", "", "", fmt.Errorf("provider %s references unknown catalog id %q", p.ID, p.ProviderID) } + switch entry.EffectiveAuthMode() { + case catalog.AuthModeNone: + return "", "", "", nil + case catalog.AuthModeOptional: + if strings.TrimSpace(p.APIKey) == "" { + return "", "", "", nil + } + case catalog.AuthModeRequired: + if strings.TrimSpace(p.APIKey) == "" { + return "", "", "", fmt.Errorf("provider %s has no api key", p.ID) + } + default: + return "", "", "", fmt.Errorf("catalog entry %q has invalid authentication mode %q", p.ProviderID, entry.AuthMode) + } if entry.AuthHeaderName == "" || entry.AuthHeaderTemplate == "" { return "", "", "", fmt.Errorf("catalog entry %q has no auth header configured", p.ProviderID) } - if p.APIKey == "" { - return "", "", "", fmt.Errorf("provider %s has no api key", p.ID) - } // A "keyfile::" api_key is a GCP service-account key, not a // static bearer. The proxy mints + refreshes a short-lived OAuth token from // it at request time, so carry the key material on the route and emit no diff --git a/management/internals/modules/agentnetwork/synthesizer_test.go b/management/internals/modules/agentnetwork/synthesizer_test.go index 206d1d12a..50fa69652 100644 --- a/management/internals/modules/agentnetwork/synthesizer_test.go +++ b/management/internals/modules/agentnetwork/synthesizer_test.go @@ -1219,3 +1219,59 @@ func TestSynthesizeServices_EmptyAPIKey_FailsClosed(t *testing.T) { require.Error(t, err, "synthesis must refuse a provider with no api key") assert.Contains(t, err.Error(), "no api key", "error must surface the missing credential") } + +func TestSynthesizeServices_OllamaOptionalAPIKey(t *testing.T) { + tests := []struct { + name string + apiKey string + wantHeaderName string + wantHeaderValue string + }{ + { + name: "no key emits no replacement auth header", + }, + { + name: "configured key emits bearer auth", + apiKey: "protected-endpoint-token", + wantHeaderName: "Authorization", + wantHeaderValue: "Bearer protected-endpoint-token", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + ctx := context.Background() + ctrl := gomock.NewController(t) + defer ctrl.Finish() + mockStore := store.NewMockStore(ctrl) + + provider := newSynthTestProvider() + provider.ProviderID = "ollama" + provider.Name = "Ollama" + provider.UpstreamURL = "http://ollama.internal:11434" + provider.APIKey = tt.apiKey + provider.Models = []types.ProviderModel{} + policy := newSynthTestPolicy(provider.ID, "grp-eng", "") + + expectSynthBaseInputs(mockStore, ctx, newSynthTestSettings(), + []*types.Provider{provider}, + []*types.Policy{policy}, + []*types.Guardrail{}) + + services, err := SynthesizeServices(ctx, mockStore, testAccountID) + require.NoError(t, err) + require.Len(t, services, 1) + + var routerCfg routerConfig + for _, middleware := range services[0].Targets[0].Options.Middlewares { + if middleware.ID == middlewareIDLLMRouter { + require.NoError(t, json.Unmarshal(middleware.ConfigJSON, &routerCfg)) + break + } + } + require.Len(t, routerCfg.Providers, 1) + assert.Equal(t, tt.wantHeaderName, routerCfg.Providers[0].AuthHeaderName) + assert.Equal(t, tt.wantHeaderValue, routerCfg.Providers[0].AuthHeaderValue) + }) + } +} diff --git a/management/internals/modules/agentnetwork/types/provider.go b/management/internals/modules/agentnetwork/types/provider.go index b3287168e..bf9401ceb 100644 --- a/management/internals/modules/agentnetwork/types/provider.go +++ b/management/internals/modules/agentnetwork/types/provider.go @@ -33,6 +33,10 @@ type Provider struct { // the operator selected. UpstreamURL string `gorm:"column:upstream_url"` APIKey string `gorm:"column:api_key"` + // APIKeyProvided records whether api_key was present in the request. It is + // transient and lets updates distinguish omission (preserve) from an + // explicit empty value (clear an optional credential). + APIKeyProvided bool `gorm:"-" json:"-"` // ExtraValues holds operator-typed values for catalog-declared // ExtraHeaders (see catalog.Provider.ExtraHeaders). Keyed by // header name (e.g. "x-portkey-config"); a non-empty value is @@ -97,15 +101,20 @@ func NewProvider(accountID string) *Provider { } } -// FromAPIRequest applies the request payload onto the receiver. The api_key -// is only overwritten when the caller provided one — empty/nil leaves the -// existing key intact, so updates can omit it. +// FromAPIRequest applies the request payload onto the receiver. APIKeyProvided +// preserves the distinction between an omitted api_key and an explicit empty +// value; the manager applies the catalog-specific preserve/clear semantics. func (p *Provider) FromAPIRequest(req *api.AgentNetworkProviderRequest) { p.ProviderID = req.ProviderId p.Name = req.Name p.UpstreamURL = req.UpstreamUrl - if req.ApiKey != nil && strings.TrimSpace(*req.ApiKey) != "" { - p.APIKey = *req.ApiKey + p.APIKeyProvided = req.ApiKey != nil + if req.ApiKey != nil { + if strings.TrimSpace(*req.ApiKey) == "" { + p.APIKey = "" + } else { + p.APIKey = *req.ApiKey + } } if req.ExtraValues != nil { // Replace the whole map (rather than merge) so unsetting a @@ -178,6 +187,7 @@ func (p *Provider) ToAPIResponse() *api.AgentNetworkProvider { UpstreamUrl: p.UpstreamURL, Models: models, Enabled: p.Enabled, + HasApiKey: strings.TrimSpace(p.APIKey) != "", SkipTlsVerification: p.SkipTLSVerification, MetadataDisabled: p.MetadataDisabled, CreatedAt: &created, diff --git a/management/internals/modules/agentnetwork/types/provider_test.go b/management/internals/modules/agentnetwork/types/provider_test.go index f9756bb8b..1678dc3b7 100644 --- a/management/internals/modules/agentnetwork/types/provider_test.go +++ b/management/internals/modules/agentnetwork/types/provider_test.go @@ -77,3 +77,37 @@ func TestProvider_MetadataDisabled_RoundTrip(t *testing.T) { assert.False(t, p.MetadataDisabled, "explicit false must clear metadata_disabled") assert.False(t, p.ToAPIResponse().MetadataDisabled, "response must reflect the cleared value") } + +func TestProvider_APIKeyPresenceAndResponse(t *testing.T) { + base := func() *api.AgentNetworkProviderRequest { + return &api.AgentNetworkProviderRequest{ + ProviderId: "ollama", + Name: "Ollama", + UpstreamUrl: "http://ollama.internal:11434", + } + } + + p := NewProvider("acc-1") + p.FromAPIRequest(base()) + assert.False(t, p.APIKeyProvided, "omission must remain distinguishable from an explicit clear") + assert.False(t, p.ToAPIResponse().HasApiKey) + + key := "protected-endpoint-token" + withKey := base() + withKey.ApiKey = &key + p.FromAPIRequest(withKey) + assert.True(t, p.APIKeyProvided) + assert.Equal(t, key, p.APIKey) + assert.True(t, p.ToAPIResponse().HasApiKey) + + empty := "" + clearKey := base() + clearKey.ApiKey = &empty + p.FromAPIRequest(clearKey) + assert.True(t, p.APIKeyProvided, "explicit empty must be carried to the manager as a clear") + assert.Empty(t, p.APIKey) + assert.False(t, p.ToAPIResponse().HasApiKey) + + p.FromAPIRequest(base()) + assert.False(t, p.APIKeyProvided, "a later omitted value must reset transient request presence") +} diff --git a/shared/management/http/api/openapi.yml b/shared/management/http/api/openapi.yml index e3d11227a..541e043e9 100644 --- a/shared/management/http/api/openapi.yml +++ b/shared/management/http/api/openapi.yml @@ -5138,6 +5138,9 @@ components: description: Models exposed through this endpoint, with the operator's per-1k input/output prices. Empty means all catalog models are allowed at catalog prices. items: $ref: '#/components/schemas/AgentNetworkProviderModel' + has_api_key: + type: boolean + description: Whether an upstream API key is currently stored. The key itself is never returned. extra_values: type: object description: | @@ -5186,6 +5189,7 @@ components: - name - upstream_url - models + - has_api_key - enabled - skip_tls_verification - metadata_disabled @@ -5212,7 +5216,8 @@ components: example: "eu.proxy.netbird.io" api_key: type: string - description: Upstream provider API key. Sealed at rest on the management server and never returned in responses. Required on create; optional on update (omit to keep the existing key). + description: | + Upstream provider API key. Sealed at rest on the management server and never returned in responses. Whether a key is accepted or required is declared by the selected catalog provider's `auth_mode`. On update, omit this field to preserve the existing key or send an empty string to clear an optional key. example: "sk-..." models: type: array @@ -5325,6 +5330,11 @@ components: type: string description: Default upstream host suggested when adding a provider of this type. example: "api.openai.com" + auth_mode: + type: string + description: Whether this provider requires, optionally accepts, or does not support an upstream API key. + enum: [required, optional, none] + example: "required" auth_header_template: type: string description: Template the proxy uses to inject the API key (the literal string ${API_KEY} is replaced at request time). @@ -5364,6 +5374,7 @@ components: - name - description - default_host + - auth_mode - auth_header_template - default_content_type - brand_color diff --git a/shared/management/http/api/types.gen.go b/shared/management/http/api/types.gen.go index a4de48a09..0eeed8709 100644 --- a/shared/management/http/api/types.gen.go +++ b/shared/management/http/api/types.gen.go @@ -38,6 +38,27 @@ func (e AccessRestrictionsCrowdsecMode) Valid() bool { } } +// Defines values for AgentNetworkCatalogProviderAuthMode. +const ( + AgentNetworkCatalogProviderAuthModeNone AgentNetworkCatalogProviderAuthMode = "none" + AgentNetworkCatalogProviderAuthModeOptional AgentNetworkCatalogProviderAuthMode = "optional" + AgentNetworkCatalogProviderAuthModeRequired AgentNetworkCatalogProviderAuthMode = "required" +) + +// Valid indicates whether the value is a known member of the AgentNetworkCatalogProviderAuthMode enum. +func (e AgentNetworkCatalogProviderAuthMode) Valid() bool { + switch e { + case AgentNetworkCatalogProviderAuthModeNone: + return true + case AgentNetworkCatalogProviderAuthModeOptional: + return true + case AgentNetworkCatalogProviderAuthModeRequired: + return true + default: + return false + } +} + // Defines values for AgentNetworkCatalogProviderKind. const ( AgentNetworkCatalogProviderKindCustom AgentNetworkCatalogProviderKind = "custom" @@ -2038,6 +2059,9 @@ type AgentNetworkCatalogProvider struct { // AuthHeaderTemplate Template the proxy uses to inject the API key (the literal string ${API_KEY} is replaced at request time). AuthHeaderTemplate string `json:"auth_header_template"` + // AuthMode Whether this provider requires, optionally accepts, or does not support an upstream API key. + AuthMode AgentNetworkCatalogProviderAuthMode `json:"auth_mode"` + // BrandColor Hex brand color used to render the provider badge in the dashboard. BrandColor string `json:"brand_color"` @@ -2072,6 +2096,9 @@ type AgentNetworkCatalogProvider struct { Name string `json:"name"` } +// AgentNetworkCatalogProviderAuthMode Whether this provider requires, optionally accepts, or does not support an upstream API key. +type AgentNetworkCatalogProviderAuthMode string + // AgentNetworkCatalogProviderKind Presentation grouping for the provider Select on the dashboard. // "provider" — first-party vendor API (OpenAI, Anthropic, …); the upstream is the model itself. // "gateway" — routing/aggregation layer in front of multiple providers (LiteLLM, Portkey, …); typically pairs with NetBird identity stamping. @@ -2260,6 +2287,9 @@ type AgentNetworkProvider struct { // ExtraValues Operator-typed values for catalog-declared extra headers. Keys are wire header names (e.g. `x-portkey-config`); values are the strings the proxy stamps on every upstream request to this provider. Catalog (AgentNetworkCatalogProvider.extra_headers) declares which keys are accepted; values not declared by the catalog are ignored at synth time. Empty / missing values mean no header stamped. ExtraValues *map[string]string `json:"extra_values,omitempty"` + // HasApiKey Whether an upstream API key is currently stored. The key itself is never returned. + HasApiKey bool `json:"has_api_key"` + // Id Provider ID Id string `json:"id"` @@ -2305,7 +2335,7 @@ type AgentNetworkProviderModel struct { // AgentNetworkProviderRequest defines model for AgentNetworkProviderRequest. type AgentNetworkProviderRequest struct { - // ApiKey Upstream provider API key. Sealed at rest on the management server and never returned in responses. Required on create; optional on update (omit to keep the existing key). + // ApiKey Upstream provider API key. Sealed at rest on the management server and never returned in responses. Whether a key is accepted or required is declared by the selected catalog provider's `auth_mode`. On update, omit this field to preserve the existing key or send an empty string to clear an optional key. ApiKey *string `json:"api_key,omitempty"` // BootstrapCluster Proxy cluster used to bootstrap the per-account agent-network endpoint when the first provider is created. Ignored on subsequent creates and on updates because the cluster is pinned on the account-level Settings row.