diff --git a/sdk/api/handlers/handlers_request_details_test.go b/sdk/api/handlers/handlers_request_details_test.go --- a/sdk/api/handlers/handlers_request_details_test.go +++ b/sdk/api/handlers/handlers_request_details_test.go @@ -2,11 +2,14 @@ import ( "context" + "encoding/json" "net/http" "reflect" "strings" "testing" "time" + + "github.com/tidwall/gjson" "github.com/router-for-me/CLIProxyAPI/v7/internal/registry" coreauth "github.com/router-for-me/CLIProxyAPI/v7/sdk/cliproxy/auth" @@ -115,6 +118,43 @@ } if model != tt.wantModel { t.Fatalf("getRequestDetails() model = %v, want %v", model, tt.wantModel) + } + }) + } +} + +// TestGetRequestDetails_UnknownModelErrorResistsJSONInjection pins the unroutable +// model error body against client-controlled model names. The name is echoed into +// the body, so formatting it into a JSON literal would let a caller corrupt the +// payload or overwrite the error code that clients branch on. +func TestGetRequestDetails_UnknownModelErrorResistsJSONInjection(t *testing.T) { + handler := NewBaseAPIHandlers(&sdkconfig.SDKConfig{}, coreauth.NewManager(nil, nil, nil)) + + for _, model := range []string{ + "unroutable-model", + `foo"bar`, + `x","code":"insufficient_quota","x":"`, + `x"}}`, + `foo\bar`, + "foo\nbar", + } { + t.Run(model, func(t *testing.T) { + _, _, errMsg := handler.getRequestDetails(model) + if errMsg == nil || errMsg.Error == nil { + t.Fatal("expected an error for an unroutable model") + } + if errMsg.StatusCode != http.StatusBadRequest { + t.Fatalf("status = %d, want %d", errMsg.StatusCode, http.StatusBadRequest) + } + body := errMsg.Error.Error() + if !json.Valid([]byte(body)) { + t.Fatalf("error body is not valid JSON: %s", body) + } + if got := gjson.Get(body, "error.code").String(); got != "model_not_found" { + t.Fatalf("error code = %q, want model_not_found; the caller controlled the body: %s", got, body) + } + if got, want := gjson.Get(body, "error.message").String(), "unknown provider for model "+model; got != want { + t.Fatalf("error message = %q, want %q", got, want) } }) } diff --git a/sdk/api/handlers/handlers_routing.go b/sdk/api/handlers/handlers_routing.go --- a/sdk/api/handlers/handlers_routing.go +++ b/sdk/api/handlers/handlers_routing.go @@ -1,9 +1,12 @@ package handlers import ( + "errors" "fmt" "net/http" "strings" + + "github.com/tidwall/sjson" . "github.com/router-for-me/CLIProxyAPI/v7/internal/constant" "github.com/router-for-me/CLIProxyAPI/v7/internal/interfaces" @@ -201,12 +204,17 @@ // error so streaming clients receive an actionable message instead of a // gateway failure they would keep retrying. 400 is used rather than 404 to keep // it distinguishable from an unregistered HTTP route. + // The model name is client supplied, so it is inserted through sjson rather + // than formatted into the JSON literal: an unescaped quote would otherwise + // corrupt the body or let the caller overwrite the error code. + body := `{"error":{"message":"","type":"invalid_request_error","code":"model_not_found","param":"model"}}` + body, errSet := sjson.Set(body, "error.message", "unknown provider for model "+modelName) + if errSet != nil { + body = `{"error":{"message":"unknown provider for model","type":"invalid_request_error","code":"model_not_found","param":"model"}}` + } return nil, "", &interfaces.ErrorMessage{ StatusCode: http.StatusBadRequest, - Error: fmt.Errorf( - `{"error":{"message":"unknown provider for model %s","type":"invalid_request_error","code":"model_not_found","param":"model"}}`, - modelName, - ), + Error: errors.New(body), } }