fix(api): bound the remaining unbounded request fields [ENG-1652]

internal/api/handlers/decode_json_handlers_test.go

@@ -297,6 +297,50 @@ func TestNewlyCappedBodiesAreBounded(t *testing.T) {
 	}
 }
 
+// Both taxonomy labels are free text with no limit of their own before ENG-1652; an over-long one
+// must be refused with 400 naming it, before the (panicking) stub service is reached.
+func TestTaxonomyLabelsAreBounded(t *testing.T) {
+	long := strings.Repeat("l", 2049)
+	id := uuid.Must(uuid.NewV7()).String()
+
+	cases := map[string]struct {
+		member string
+		body   string
+		call   func(http.ResponseWriter, *http.Request)
+	}{
+		"taxonomy run create": {
+			member: "field_label",
+			body: `{"tenant_id":"tenant-a","source_type":"survey","source_id":"s","field_id":"q1",` +
+				`"actor_id":"a","field_label":"` + long + `"}`,
+			call: NewTaxonomyHandler(&stubTaxonomyService{}).CreateRun,
+		},
+		"taxonomy node rename": {
+			member: "label",
+			body:   `{"tenant_id":"tenant-a","actor_id":"a","label":"` + long + `"}`,
+			call:   NewTaxonomyHandler(&stubTaxonomyService{}).RenameNode,
+		},
+	}
+
+	for name, testCase := range cases {
+		t.Run(name, func(t *testing.T) {
+			req := httptest.NewRequestWithContext(context.Background(), http.MethodPost, "http://test/x",
+				strings.NewReader(testCase.body))
+			req.SetPathValue("node_id", id)
+
+			rec := httptest.NewRecorder()
+
+			require.NotPanics(t, func() { testCase.call(rec, req) }, "the body must be refused before any service call")
+
+			assert.Equal(t, http.StatusBadRequest, rec.Code)
+
+			var problem response.ProblemDetails
+			require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &problem))
+			require.Len(t, problem.InvalidParams, 1)
+			assert.Equal(t, testCase.member, problem.InvalidParams[0].Name)
+		})
+	}
+}
+
 type recordingWebhooksService struct{ called bool }
 
 func (s *recordingWebhooksService) CreateWebhook(context.Context, *models.CreateWebhookRequest) (*models.Webhook, error) {

internal/api/handlers/search_handler.go

@@ -34,9 +34,11 @@ func NewSearchHandler(service SearchService) *SearchHandler {
 }
 
 // SemanticSearchRequest is the body for POST /v1/feedback-records/search/semantic (snake_case for consistency with data model).
+// The query is sent to the paid embedding provider on every uncached call, so it is capped; 4,096
+// characters is twice what the Formbricks gateway already allows, so the gateway's error comes first.
 type SemanticSearchRequest struct {
-	Query    string `json:"query"`
-	TenantID string `json:"tenant_id" validate:"required,no_replacement_char"`
+	Query    string `json:"query"     validate:"max=4096"`
+	TenantID string `json:"tenant_id" validate:"required,no_replacement_char,max=255"`
 }
 
 // SemanticSearchResponse is the response for semantic search and similar feedback (consistent with list endpoints: data, limit).

internal/api/handlers/search_handler_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"strings"
 	"testing"
 
 	"github.com/google/uuid"
@@ -81,6 +82,62 @@ func TestSearchHandler_SemanticSearch(t *testing.T) {
 		assert.Equal(t, http.StatusBadRequest, rec.Code)
 	})
 
+	t.Run("over-long query or tenant_id returns 400 before the provider is called", func(t *testing.T) {
+		for name, body := range map[string]map[string]string{
+			"query":     {"query": strings.Repeat("a", 4097), "tenant_id": "env-1"},
+			"tenant_id": {"query": "login is slow", "tenant_id": strings.Repeat("t", 256)},
+		} {
+			t.Run(name, func(t *testing.T) {
+				called := false
+				mock := &mockSearchService{
+					semanticFunc: func(_ context.Context, _, _ string, _ int, _ float64, _ string) (service.SearchResult, error) {
+						called = true
+
+						return service.SearchResult{}, nil
+					},
+				}
+				raw, err := json.Marshal(body)
+				require.NoError(t, err)
+
+				req := httptest.NewRequestWithContext(context.Background(),
+					http.MethodPost, "http://test/v1/feedback-records/search/semantic", bytes.NewReader(raw))
+				req.Header.Set("Content-Type", "application/json")
+
+				rec := httptest.NewRecorder()
+
+				NewSearchHandler(mock).SemanticSearch(rec, req)
+
+				assert.Equal(t, http.StatusBadRequest, rec.Code)
+				assert.Contains(t, rec.Body.String(), `"name":"`+name+`"`)
+				assert.False(t, called)
+			})
+		}
+	})
+
+	t.Run("query at the limit counts characters, not bytes", func(t *testing.T) {
+		called := false
+		mock := &mockSearchService{
+			semanticFunc: func(_ context.Context, _, _ string, _ int, _ float64, _ string) (service.SearchResult, error) {
+				called = true
+
+				return service.SearchResult{}, nil
+			},
+		}
+		raw, err := json.Marshal(map[string]string{"query": strings.Repeat("ü", 4096), "tenant_id": "env-1"})
+		require.NoError(t, err)
+
+		req := httptest.NewRequestWithContext(context.Background(),
+			http.MethodPost, "http://test/v1/feedback-records/search/semantic", bytes.NewReader(raw))
+		req.Header.Set("Content-Type", "application/json")
+
+		rec := httptest.NewRecorder()
+
+		NewSearchHandler(mock).SemanticSearch(rec, req)
+
+		assert.Equal(t, http.StatusOK, rec.Code)
+		assert.True(t, called)
+	})
+
 	t.Run("success returns 200 with data and value", func(t *testing.T) {
 		id1 := uuid.MustParse("018e1234-5678-9abc-def0-111111111111")
 		id2 := uuid.MustParse("018e1234-5678-9abc-def0-222222222222")

internal/api/response/errors.go

@@ -154,11 +154,13 @@ const (
 	ReasonJSONMemberRepeated = "appears more than once; each member may appear only once"
 )
 
-// maxReportedJSONNameRunes bounds how much of a caller-chosen member path is echoed back, and
-// maxReportedJSONReasonRunes how much of a decoder's reason, which can quote the caller's value.
+// maxReportedJSONNameRunes bounds how much of a caller-chosen member path is echoed back,
+// maxReportedJSONReasonRunes how much of a decoder's reason, which can quote the caller's value,
+// and maxReportedValueRunes how much of a rejected value a reason quotes itself.
 const (
 	maxReportedJSONNameRunes   = 64
 	maxReportedJSONReasonRunes = 256
+	maxReportedValueRunes      = 64
 )
 
 // problemFromJSONDecodeError recognizes errors from decoding a JSON request body with
@@ -363,7 +365,7 @@ func invalidFieldTypeParam(err error) (InvalidParam, bool) {
 		Name: "field_type",
 		Reason: fmt.Sprintf(
 			"has invalid value %q; must be one of: %s",
-			invalidFieldType.Value,
+			truncateRunes(invalidFieldType.Value, maxReportedValueRunes),
 			models.ValidFieldTypeValuesString(),
 		),
 	}, true

internal/api/response/response_test.go

@@ -158,6 +158,21 @@ func TestRespondErrorInvalidFieldTypeReason(t *testing.T) {
 	assert.Contains(t, problem.InvalidParams[0].Reason, "date")
 }
 
+func TestRespondErrorInvalidFieldTypeReasonTruncatesLongValue(t *testing.T) {
+	rec := httptest.NewRecorder()
+	value := strings.Repeat("x", 512<<10)
+
+	RespondError(rec, newReq(t, http.MethodPost, "/v1/feedback-records"), &models.InvalidFieldTypeError{Value: value})
+
+	problem := decodeProblem(t, rec)
+	require.Len(t, problem.InvalidParams, 1)
+
+	reason := problem.InvalidParams[0].Reason
+	assert.Contains(t, reason, `"`+strings.Repeat("x", maxReportedValueRunes)+`…"`)
+	assert.NotContains(t, reason, strings.Repeat("x", maxReportedValueRunes+1))
+	assert.Contains(t, reason, "must be one of")
+}
+
 func TestRespondErrorQueryDecodeErrorIsValidationProblem(t *testing.T) {
 	var filters struct {
 		Since *time.Time `form:"since"`

internal/models/feedback_records_filters_openapi_test.go

@@ -127,6 +127,20 @@ func TestOpenAPICapsMatchTheFilterStruct(t *testing.T) {
 func loadOpenAPIParameters(t *testing.T) openAPIParameters {
 	t.Helper()
 
+	var spec openAPIParameters
+	loadOpenAPISpec(t, &spec)
+
+	if len(spec.Components.Parameters) == 0 {
+		t.Fatal("openapi.yaml parsed to zero parameters; this test would assert nothing")
+	}
+
+	return spec
+}
+
+// loadOpenAPISpec decodes the repository's openapi.yaml into dst, which names only the parts a test needs.
+func loadOpenAPISpec(t *testing.T, dst any) {
+	t.Helper()
+
 	_, filename, _, ok := runtime.Caller(0)
 	if !ok {
 		t.Fatal("cannot resolve this test file's path")
@@ -139,16 +153,9 @@ func loadOpenAPIParameters(t *testing.T) openAPIParameters {
 		t.Fatalf("read openapi.yaml: %v", err)
 	}
 
-	var spec openAPIParameters
-	if err := yaml.Unmarshal(contents, &spec); err != nil {
+	if err := yaml.Unmarshal(contents, dst); err != nil {
 		t.Fatalf("parse openapi.yaml: %v", err)
 	}
-
-	if len(spec.Components.Parameters) == 0 {
-		t.Fatal("openapi.yaml parsed to zero parameters; this test would assert nothing")
-	}
-
-	return spec
 }
 
 // openAPIOperationParameters is the slice of openapi.yaml naming each list operation's query

internal/models/request_arrays_openapi_test.go

@@ -0,0 +1,66 @@
+package models
+
+import (
+	"testing"
+
+	"github.com/formbricks/hub/internal/datatypes"
+)
+
+type openAPIRequestSchemas struct {
+	Components struct {
+		Schemas map[string]struct {
+			Properties map[string]struct {
+				MaxItems *int `yaml:"maxItems"`
+				Items    struct {
+					Enum []string `yaml:"enum"`
+				} `yaml:"items"`
+			} `yaml:"properties"`
+		} `yaml:"schemas"`
+	} `yaml:"components"`
+}
+
+// TestOpenAPIRequestArrayCapsMatchTheirValues pins each request array whose values come from a fixed
+// set to a maxItems equal to that set's size. The server refuses a repeated or unknown value, so the
+// set's size is the real bound; a spec that falls behind it (a seventh event type added, maxItems left
+// at six) would make generated clients refuse a list the server accepts.
+func TestOpenAPIRequestArrayCapsMatchTheirValues(t *testing.T) {
+	t.Parallel()
+
+	var spec openAPIRequestSchemas
+	loadOpenAPISpec(t, &spec)
+
+	retry := spec.Components.Schemas["EnrichmentRetryInputBody"].Properties["enrichments"]
+
+	cases := map[string]struct {
+		maxItems *int
+		want     int
+	}{
+		"CreateWebhookInputBody.event_types": {
+			spec.Components.Schemas["CreateWebhookInputBody"].Properties["event_types"].MaxItems,
+			len(datatypes.GetAllEventTypes()),
+		},
+		"UpdateWebhookInputBody.event_types": {
+			spec.Components.Schemas["UpdateWebhookInputBody"].Properties["event_types"].MaxItems,
+			len(datatypes.GetAllEventTypes()),
+		},
+		"EnrichmentRetryInputBody.enrichments": {retry.MaxItems, len(retry.Items.Enum)},
+	}
+
+	for name, testCase := range cases {
+		if testCase.maxItems == nil {
+			t.Errorf("%s has no maxItems in openapi.yaml", name)
+
+			continue
+		}
+
+		if testCase.want == 0 {
+			t.Errorf("%s: found no allowed values to compare against; this check would assert nothing", name)
+
+			continue
+		}
+
+		if *testCase.maxItems != testCase.want {
+			t.Errorf("%s: maxItems is %d, but there are %d allowed values", name, *testCase.maxItems, testCase.want)
+		}
+	}
+}

internal/models/taxonomy.go

@@ -131,7 +131,7 @@ type TaxonomyRun struct {
 type CreateTaxonomyRunRequest struct {
 	TaxonomyScope
 
-	FieldLabel *string `json:"field_label,omitempty" validate:"omitempty,no_null_bytes"`
+	FieldLabel *string `json:"field_label,omitempty" validate:"omitempty,no_null_bytes,max=2048"`
 	ActorID    *string `json:"actor_id,omitempty"    validate:"omitempty,no_null_bytes,min=1,max=255"`
 }
 
@@ -339,7 +339,7 @@ type TaxonomyRunPartialMetrics struct {
 type RenameTaxonomyNodeRequest struct {
 	TenantID string `json:"tenant_id" validate:"required,no_null_bytes,no_replacement_char,min=1,max=255"`
 	ActorID  string `json:"actor_id"  validate:"required,no_null_bytes,min=1,max=255"`
-	Label    string `json:"label"     validate:"required,no_null_bytes,min=1"`
+	Label    string `json:"label"     validate:"required,no_null_bytes,min=1,max=2048"`
 }
 
 // TaxonomyNodeRecordsFilters scopes taxonomy node feedback record drilldown.

internal/service/enrichment_retry_service.go

@@ -255,6 +255,13 @@ func normalizeRequestedEnrichments(requested []string) ([]string, error) {
 		return all, nil
 	}
 
+	// There are only three names, so a longer list can only repeat them. The cap matches the spec's
+	// maxItems, which lets a generated client refuse such a list before sending it.
+	if len(requested) > len(all) {
+		return nil, huberrors.NewValidationError("enrichments",
+			fmt.Sprintf("must list at most %d enrichments", len(all)))
+	}
+
 	known := make(map[string]struct{}, len(all))
 	for _, name := range all {
 		known[name] = struct{}{}

internal/service/enrichment_retry_service_test.go

@@ -85,6 +85,25 @@ func TestRetryReportsTheCeilingOfTheWait(t *testing.T) {
 		"90.4s remaining must report 91, not 90")
 }
 
+// TestRetryRefusesMoreNamesThanThereAreEnrichments: a repeated name is still folded away, but a
+// list longer than the three enrichments is refused before anything is cleared.
+func TestRetryRefusesMoreNamesThanThereAreEnrichments(t *testing.T) {
+	repo := &fakeRetryRepo{claimed: true, cleared: 1}
+	svc := newRetryService(repo, true)
+
+	resp, err := svc.Retry(context.Background(), "tenant-1", []string{"sentiment", "sentiment"})
+	require.NoError(t, err)
+	assert.Len(t, resp.Results, 1)
+
+	_, err = svc.Retry(context.Background(), "tenant-1", []string{"sentiment", "emotions", "translation", "sentiment"})
+	require.Error(t, err)
+
+	var validation *huberrors.ValidationError
+	require.ErrorAs(t, err, &validation)
+	assert.Equal(t, "enrichments", validation.Field)
+	assert.Equal(t, 1, repo.calls, "only the valid request reached the repository")
+}
+
 // TestRetryWindowDecisionBelongsToTheClear: the service must not pre-check the cooldown and then
 // clear — the repository claims the window atomically with the delete, and the service's only job
 // on refusal is to report the wait. One repo call on the success path is the proof.