diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler.go b/management/internals/modules/agentnetwork/handlers/providers_handler.go index 645d1da61..64b0e4e0a 100644 --- a/management/internals/modules/agentnetwork/handlers/providers_handler.go +++ b/management/internals/modules/agentnetwork/handlers/providers_handler.go @@ -339,6 +339,14 @@ func validate(req *api.AgentNetworkProviderRequest, requireAPIKey bool) error { if requireAPIKey && (req.ApiKey == nil || strings.TrimSpace(*req.ApiKey) == "") { return status.Errorf(status.InvalidArgument, "api_key is required") } + // An update omits api_key to keep the stored credential. A key that is + // present but blank is not that: Provider.FromAPIRequest drops it exactly + // as if it were absent, so a rotation the operator believes they performed + // would answer 200 having changed nothing. Refuse it here, where the + // request still carries the difference between absent and blank. + if req.ApiKey != nil && strings.TrimSpace(*req.ApiKey) == "" { + return status.Errorf(status.InvalidArgument, "api_key must be omitted to keep the stored credential rather than sent blank") + } if req.Models != nil { for i, m := range *req.Models { if err := validateModel(i, m); err != nil { diff --git a/management/internals/modules/agentnetwork/handlers/providers_handler_test.go b/management/internals/modules/agentnetwork/handlers/providers_handler_test.go index c32835a56..033de3b8a 100644 --- a/management/internals/modules/agentnetwork/handlers/providers_handler_test.go +++ b/management/internals/modules/agentnetwork/handlers/providers_handler_test.go @@ -54,6 +54,39 @@ func TestValidate_ModelRates(t *testing.T) { } } +// TestValidate_ABlankApiKeyIsNotTheSameAsAnOmittedOne covers the one shape the +// manager's own guard cannot see. Provider.FromAPIRequest assigns the key only +// when it trims to something, so a request carrying " " arrives at +// UpdateProvider indistinguishable from one that omitted it — the stored +// credential is kept and the write answers 200, telling an operator who thinks +// they just rotated a key that it worked. +// +// The request still knows the difference, so the refusal belongs here. +func TestValidate_ABlankApiKeyIsNotTheSameAsAnOmittedOne(t *testing.T) { + req := func(key *string) *api.AgentNetworkProviderRequest { + return &api.AgentNetworkProviderRequest{ + ProviderId: "openai_api", + Name: "OpenAI", + UpstreamUrl: "https://api.openai.com", + ApiKey: key, + } + } + + blank := " " + err := validate(req(&blank), false) + require.Error(t, err, "a blank api_key on update must not be read as 'keep what is stored'") + assert.Contains(t, err.Error(), "api_key") + + require.NoError(t, validate(req(nil), false), "an omitted api_key is how an update keeps the stored credential") + + // Create already refuses this, and keeps its own message: a caller who sent + // no usable key is told the field is required rather than being told how to + // preserve a credential that does not exist yet. + err = validate(req(&blank), true) + require.Error(t, err) + assert.Contains(t, err.Error(), "api_key is required") +} + // TestProviderHandler_UpdateReplacesFullState pins the update contract shared // with the other PUT endpoints: the request replaces the provider's mutable // state, so optional fields absent from the JSON land as their zero values.