From 94a7ce695eb6035275eacebd770b5d49dce8582a Mon Sep 17 00:00:00 2001 From: matthew-hull-bright Date: Fri, 11 Sep 2026 11:23:05 +0100 Subject: [PATCH 1/6] add tests to detect bug --- terraform/provider/litellm/client_test.go | 57 +++++++++++++ terraform/provider/litellm/utils_test.go | 98 +++++++++++++++++++++++ 2 files changed, 155 insertions(+) create mode 100644 terraform/provider/litellm/utils_test.go diff --git a/terraform/provider/litellm/client_test.go b/terraform/provider/litellm/client_test.go index 56f76565616..77c7bf817d8 100644 --- a/terraform/provider/litellm/client_test.go +++ b/terraform/provider/litellm/client_test.go @@ -1,10 +1,67 @@ package litellm import ( + "errors" + "net/http" + "net/http/httptest" "strings" "testing" ) +func TestSendRequestAcceptsFullSuccessRange(t *testing.T) { + tests := []struct { + name string + statusCode int + wantErr bool + wantValue string + }{ + {name: "200 OK", statusCode: http.StatusOK, wantErr: false, wantValue: "ok"}, + {name: "201 Created", statusCode: http.StatusCreated, wantErr: false, wantValue: "created"}, + {name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false, wantValue: "accepted"}, + {name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true}, + {name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true}, + {name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(tt.statusCode) + w.Write([]byte(`{"value":"` + tt.wantValue + `"}`)) + })) + defer srv.Close() + + client := NewClient(srv.URL, "test-key", true) + result, err := client.sendRequest("POST", "/whatever", map[string]string{"foo": "bar"}) + + if tt.wantErr { + if err == nil { + t.Fatalf("sendRequest returned no error for status %d", tt.statusCode) + } + var apiErr *apiError + if !errors.As(err, &apiErr) { + t.Fatalf("error is not *apiError: %v", err) + } + if apiErr.StatusCode != tt.statusCode { + t.Errorf("apiErr.StatusCode = %d, want %d", apiErr.StatusCode, tt.statusCode) + } + if tt.statusCode == http.StatusNotFound && !isNotFound(err) { + t.Errorf("isNotFound(err) = false for 404, want true") + } + return + } + + if err != nil { + t.Fatalf("sendRequest returned unexpected error: %v", err) + } + if result["value"] != tt.wantValue { + t.Errorf("result[value] = %v, want %q", result["value"], tt.wantValue) + } + }) + } +} + func TestRedactSensitiveDataNestedCredentialValues(t *testing.T) { c := NewClient("http://localhost:4000", "sk-test", false) diff --git a/terraform/provider/litellm/utils_test.go b/terraform/provider/litellm/utils_test.go new file mode 100644 index 00000000000..e942dde288f --- /dev/null +++ b/terraform/provider/litellm/utils_test.go @@ -0,0 +1,98 @@ +package litellm + +import ( + "net/http" + "net/http/httptest" + "strconv" + "strings" + "testing" +) + +func TestHandleAPIResponseAcceptsFullSuccessRange(t *testing.T) { + tests := []struct { + name string + statusCode int + wantErr bool + }{ + {name: "200 OK", statusCode: http.StatusOK, wantErr: false}, + {name: "201 Created", statusCode: http.StatusCreated, wantErr: false}, + {name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false}, + {name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true}, + {name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true}, + {name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + rec.WriteHeader(tt.statusCode) + rec.WriteString(`{"model_name":"gpt-4o"}`) + resp := rec.Result() + + client := NewClient("http://localhost:4000", "test-key", true) + got, err := handleAPIResponse(resp, map[string]interface{}{"model_name": "gpt-4o"}, client) + + if tt.wantErr { + if err == nil { + t.Fatalf("handleAPIResponse returned no error for status %d", tt.statusCode) + } + if !strings.Contains(err.Error(), strconv.Itoa(tt.statusCode)) { + t.Errorf("error %q does not mention status %d", err.Error(), tt.statusCode) + } + return + } + + if err != nil { + t.Fatalf("handleAPIResponse returned unexpected error: %v", err) + } + if got.ModelName != "gpt-4o" { + t.Errorf("got.ModelName = %q, want gpt-4o", got.ModelName) + } + }) + } +} + +func TestHandleMCPAPIResponseAcceptsFullSuccessRange(t *testing.T) { + tests := []struct { + name string + statusCode int + wantErr bool + }{ + {name: "200 OK", statusCode: http.StatusOK, wantErr: false}, + {name: "201 Created", statusCode: http.StatusCreated, wantErr: false}, + {name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false}, + {name: "400 Bad Request", statusCode: http.StatusBadRequest, wantErr: true}, + {name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true}, + {name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + rec.WriteHeader(tt.statusCode) + rec.WriteString(`{"server_id":"srv-1","server_name":"gh"}`) + resp := rec.Result() + + client := NewClient("http://localhost:4000", "test-key", true) + var mcpResp MCPServerResponse + err := handleMCPAPIResponse(resp, &mcpResp, client) + + if tt.wantErr { + if err == nil { + t.Fatalf("handleMCPAPIResponse returned no error for status %d", tt.statusCode) + } + if !strings.Contains(err.Error(), strconv.Itoa(tt.statusCode)) { + t.Errorf("error %q does not mention status %d", err.Error(), tt.statusCode) + } + return + } + + if err != nil { + t.Fatalf("handleMCPAPIResponse returned unexpected error: %v", err) + } + if mcpResp.ServerID != "srv-1" { + t.Errorf("mcpResp.ServerID = %q, want srv-1", mcpResp.ServerID) + } + }) + } +} From 27764b929f5973bdf45a3fa66c599ef8f7ea49ea Mon Sep 17 00:00:00 2001 From: matthew-hull-bright Date: Fri, 11 Sep 2026 12:44:33 +0100 Subject: [PATCH 2/6] fix/allow 2xx status codes in client and utils --- terraform/provider/litellm/client.go | 2 +- terraform/provider/litellm/utils.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/terraform/provider/litellm/client.go b/terraform/provider/litellm/client.go index e68b8a3a80b..04473be32fb 100644 --- a/terraform/provider/litellm/client.go +++ b/terraform/provider/litellm/client.go @@ -422,7 +422,7 @@ func (c *Client) sendRequest(method, path string, body interface{}) (map[string] log.Printf("Response status: %d", resp.StatusCode) log.Printf("Response body: %s", c.redactSensitiveData(string(bodyBytes))) - if resp.StatusCode != http.StatusOK { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return nil, &apiError{StatusCode: resp.StatusCode, Body: string(bodyBytes)} } diff --git a/terraform/provider/litellm/utils.go b/terraform/provider/litellm/utils.go index 5e81766d3f3..88ef74d78e8 100644 --- a/terraform/provider/litellm/utils.go +++ b/terraform/provider/litellm/utils.go @@ -42,7 +42,7 @@ func handleAPIResponse(resp *http.Response, reqBody interface{}, client *Client) return nil, fmt.Errorf("failed to read response body: %v", err) } - if resp.StatusCode != http.StatusOK { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { var errResp ErrorResponse if err := json.Unmarshal(bodyBytes, &errResp); err == nil { if isModelNotFoundError(errResp) { @@ -132,7 +132,7 @@ func handleMCPAPIResponse(resp *http.Response, result interface{}, client *Clien return fmt.Errorf("failed to read response body: %v", err) } - if resp.StatusCode != http.StatusOK { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { var errResp ErrorResponse if err := json.Unmarshal(bodyBytes, &errResp); err == nil { if isMCPServerNotFoundError(errResp) { From cfb0853c5b72734705ab81c47e2080f28ec6e1c8 Mon Sep 17 00:00:00 2001 From: matthew-hull-bright Date: Fri, 11 Sep 2026 13:11:15 +0100 Subject: [PATCH 3/6] changelog --- terraform/provider/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/terraform/provider/CHANGELOG.md b/terraform/provider/CHANGELOG.md index e5e0a164a83..55e86589b74 100644 --- a/terraform/provider/CHANGELOG.md +++ b/terraform/provider/CHANGELOG.md @@ -43,6 +43,7 @@ longer signal it. - **key**: Updates no longer send an empty `budget_duration`, which the proxy rejects with a 400; any update to a key without a configured `budget_duration` previously failed outright - **key**: A config-supplied `key` value (write-only) is now forwarded to `/key/generate`; previously it was silently dropped and the proxy generated a random key instead - **security**: The `litellm_key` data source and `litellm_key_block` resource normalize raw `sk-` keys to their SHA-256 token hash before building request URLs and resource IDs, so plaintext keys no longer land in reverse-proxy access logs, Terraform plan output, or state IDs +- **mcp_server, model, key, organization_member**: Requests now accept any 2xx response instead of requiring exactly HTTP 200; the proxy legitimately returns 201 from `POST /v1/mcp/server` (and other create endpoints), so `litellm_mcp_server` create previously succeeded on the proxy but failed in the provider, leaving the resource out of state and risking a duplicate server on retry ### Changed From 98fb823dc2d296a06d60fcc3a885f4077234ff99 Mon Sep 17 00:00:00 2001 From: matthew-hull-bright Date: Fri, 11 Sep 2026 13:41:47 +0100 Subject: [PATCH 4/6] fix/mcp resource deletion --- .../litellm/resource_mcp_server_crud.go | 2 +- .../litellm/resource_mcp_server_crud_test.go | 48 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/terraform/provider/litellm/resource_mcp_server_crud.go b/terraform/provider/litellm/resource_mcp_server_crud.go index 2a8980960f1..1c956d88f69 100644 --- a/terraform/provider/litellm/resource_mcp_server_crud.go +++ b/terraform/provider/litellm/resource_mcp_server_crud.go @@ -270,7 +270,7 @@ func resourceLiteLLMMCPServerDelete(d *schema.ResourceData, m interface{}) error defer resp.Body.Close() // For delete operations, we expect a simple string response - if resp.StatusCode != 200 { + if resp.StatusCode < 200 || resp.StatusCode >= 300 { return fmt.Errorf("failed to delete MCP server: unexpected status code %d", resp.StatusCode) } diff --git a/terraform/provider/litellm/resource_mcp_server_crud_test.go b/terraform/provider/litellm/resource_mcp_server_crud_test.go index 17300701954..8c67fbe6405 100644 --- a/terraform/provider/litellm/resource_mcp_server_crud_test.go +++ b/terraform/provider/litellm/resource_mcp_server_crud_test.go @@ -1,11 +1,59 @@ package litellm import ( + "net/http" + "net/http/httptest" "testing" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" ) +func TestMCPServerDeleteAcceptsFullSuccessRange(t *testing.T) { + tests := []struct { + name string + statusCode int + wantErr bool + }{ + {name: "200 OK", statusCode: http.StatusOK, wantErr: false}, + {name: "202 Accepted", statusCode: http.StatusAccepted, wantErr: false}, + {name: "404 Not Found", statusCode: http.StatusNotFound, wantErr: true}, + {name: "500 Internal Server Error", statusCode: http.StatusInternalServerError, wantErr: true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(tt.statusCode) + })) + defer srv.Close() + + d := schema.TestResourceDataRaw(t, resourceLiteLLMMCPServer().Schema, map[string]interface{}{ + "server_name": "gh", + "transport": "stdio", + "command": "npx", + }) + d.SetId("srv-1") + + client := NewClient(srv.URL, "test-key", true) + err := resourceLiteLLMMCPServerDelete(d, client) + + if tt.wantErr { + if err == nil { + t.Fatalf("resourceLiteLLMMCPServerDelete returned no error for status %d", tt.statusCode) + } + return + } + + if err != nil { + t.Fatalf("resourceLiteLLMMCPServerDelete returned unexpected error: %v", err) + } + if d.Id() != "" { + t.Errorf("resource ID not cleared after successful delete, got %q", d.Id()) + } + }) + } +} + func TestMCPServerReadDoesNotPersistServerEnv(t *testing.T) { d := schema.TestResourceDataRaw(t, resourceLiteLLMMCPServer().Schema, map[string]interface{}{ "server_name": "gh", From 9d02b4546f88ebcfc1b1e6b5a517e4efae0417ac Mon Sep 17 00:00:00 2001 From: matthew-hull-bright Date: Fri, 11 Sep 2026 15:17:26 +0100 Subject: [PATCH 5/6] fix/allow bodyless 2xx --- terraform/provider/litellm/utils.go | 8 +++++++ terraform/provider/litellm/utils_test.go | 30 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/terraform/provider/litellm/utils.go b/terraform/provider/litellm/utils.go index 88ef74d78e8..5033285e824 100644 --- a/terraform/provider/litellm/utils.go +++ b/terraform/provider/litellm/utils.go @@ -54,6 +54,10 @@ func handleAPIResponse(resp *http.Response, reqBody interface{}, client *Client) resp.Status, client.redactSensitiveData(string(bodyBytes)), client.redactSensitiveData(string(reqBodyBytes))) } + if len(bodyBytes) == 0 || string(bodyBytes) == "null" { + return &ModelResponse{}, nil + } + var modelResp ModelResponse if err := json.Unmarshal(bodyBytes, &modelResp); err != nil { return nil, fmt.Errorf("failed to parse response: %v", err) @@ -143,6 +147,10 @@ func handleMCPAPIResponse(resp *http.Response, result interface{}, client *Clien resp.Status, client.redactSensitiveData(string(bodyBytes))) } + if len(bodyBytes) == 0 || string(bodyBytes) == "null" { + return nil + } + if err := json.Unmarshal(bodyBytes, result); err != nil { return fmt.Errorf("failed to parse response: %v", err) } diff --git a/terraform/provider/litellm/utils_test.go b/terraform/provider/litellm/utils_test.go index e942dde288f..2a3f5edb701 100644 --- a/terraform/provider/litellm/utils_test.go +++ b/terraform/provider/litellm/utils_test.go @@ -52,6 +52,22 @@ func TestHandleAPIResponseAcceptsFullSuccessRange(t *testing.T) { } } +func TestHandleAPIResponseAcceptsEmptyBodyOn2xx(t *testing.T) { + rec := httptest.NewRecorder() + rec.WriteHeader(http.StatusNoContent) + resp := rec.Result() + + client := NewClient("http://localhost:4000", "test-key", true) + got, err := handleAPIResponse(resp, map[string]interface{}{"model_name": "gpt-4o"}, client) + + if err != nil { + t.Fatalf("handleAPIResponse returned unexpected error for empty-body 204: %v", err) + } + if got == nil { + t.Fatal("handleAPIResponse returned nil ModelResponse for empty-body 204") + } +} + func TestHandleMCPAPIResponseAcceptsFullSuccessRange(t *testing.T) { tests := []struct { name string @@ -96,3 +112,17 @@ func TestHandleMCPAPIResponseAcceptsFullSuccessRange(t *testing.T) { }) } } + +func TestHandleMCPAPIResponseAcceptsEmptyBodyOn2xx(t *testing.T) { + rec := httptest.NewRecorder() + rec.WriteHeader(http.StatusNoContent) + resp := rec.Result() + + client := NewClient("http://localhost:4000", "test-key", true) + var mcpResp MCPServerResponse + err := handleMCPAPIResponse(resp, &mcpResp, client) + + if err != nil { + t.Fatalf("handleMCPAPIResponse returned unexpected error for empty-body 204: %v", err) + } +} From 825b092015e1d4f7bb1fa637b7397c539dca34b3 Mon Sep 17 00:00:00 2001 From: matthew-hull-bright Date: Fri, 11 Sep 2026 15:37:07 +0100 Subject: [PATCH 6/6] fix(terraform): reject empty-body 2xx responses in mcp server handler --- terraform/provider/litellm/utils.go | 4 ---- terraform/provider/litellm/utils_test.go | 6 +++--- 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/terraform/provider/litellm/utils.go b/terraform/provider/litellm/utils.go index 5033285e824..eb68f919d00 100644 --- a/terraform/provider/litellm/utils.go +++ b/terraform/provider/litellm/utils.go @@ -147,10 +147,6 @@ func handleMCPAPIResponse(resp *http.Response, result interface{}, client *Clien resp.Status, client.redactSensitiveData(string(bodyBytes))) } - if len(bodyBytes) == 0 || string(bodyBytes) == "null" { - return nil - } - if err := json.Unmarshal(bodyBytes, result); err != nil { return fmt.Errorf("failed to parse response: %v", err) } diff --git a/terraform/provider/litellm/utils_test.go b/terraform/provider/litellm/utils_test.go index 2a3f5edb701..10d6a11594a 100644 --- a/terraform/provider/litellm/utils_test.go +++ b/terraform/provider/litellm/utils_test.go @@ -113,7 +113,7 @@ func TestHandleMCPAPIResponseAcceptsFullSuccessRange(t *testing.T) { } } -func TestHandleMCPAPIResponseAcceptsEmptyBodyOn2xx(t *testing.T) { +func TestHandleMCPAPIResponseRejectsEmptyBodyOn2xx(t *testing.T) { rec := httptest.NewRecorder() rec.WriteHeader(http.StatusNoContent) resp := rec.Result() @@ -122,7 +122,7 @@ func TestHandleMCPAPIResponseAcceptsEmptyBodyOn2xx(t *testing.T) { var mcpResp MCPServerResponse err := handleMCPAPIResponse(resp, &mcpResp, client) - if err != nil { - t.Fatalf("handleMCPAPIResponse returned unexpected error for empty-body 204: %v", err) + if err == nil { + t.Fatal("handleMCPAPIResponse returned no error for empty-body 204; every MCP caller (create/read/update) writes the parsed result straight into Terraform state with no fallback, so a silently-accepted empty body would blank out or empty-ID the resource") } }