[management] Refuse a blank api_key on a provider update instead of ignoring it

This commit is contained in:
mlsmaycon
2026-08-27 08:48:13 +00:00
parent e85e18e5b9
commit 7ca9be2f2f
2 changed files with 41 additions and 0 deletions

View File

@@ -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 {

View File

@@ -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.