Skip to content

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

Merged
pandeymangg merged 1 commit into
mainfrom
anshuman/eng-1652-hub-request-field-limits
Oct 16, 2026
Merged

pandeymangg merged 1 commit into
mainfrom
anshuman/eng-1652-hub-request-field-limits

Conversation

@pandeymangg

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes ENG-1652

Every request body is already capped (#141). This gives the fields that still had no limit of their own an explicit one, in Go and in openapi.yaml, so generated SDKs check them client-side too.

Field Limit Why this number
Semantic search query 4,096 chars Sent to the paid embedding provider; 2× what the Formbricks v3 API allows (2,000)
Semantic search tenant_id 255 Same as every other tenant_id
Taxonomy run field_label 2,048 Same as a feedback record's field_label
Taxonomy node rename label 2,048 Same; the Formbricks v3 API allows 1,000
Webhook event_types (create, update) maxItems: 6, uniqueItems Spec only: the server already refused repeats and unknown values
Enrichment retry enrichments 3 items Only three names exist, so a longer list could only repeat them

Also, a rejected field_type is now quoted up to 64 characters in invalid_params, instead of echoing the whole value (up to ~512 KiB) back.

Not in this PR: metrics on the internal CompleteRun. The caller is trusted, it is already under the 16 MiB body cap, and real runs' size isn't measured, so a cap could fail them. The enrichment input budget moved to ENG-3908.

POST /v1/feedback-records/search/semantic
{"query":"aaaa… (4,097 characters)","tenant_id":"org-123"}

400 {"type":"https://hub.formbricks.com/problems/validation","title":"Validation Error","status":400,
     "code":"validation","invalid_params":[{"name":"query","reason":"must be at most 4096"}], …}

📝 Release note

Behavior change: more request fields are bounded. These were accepted before and now return 400: a semantic search query over 4,096 characters or tenant_id over 255; a taxonomy run field_label or node rename label over 2,048; an enrichment retry listing more than 3 names. OpenAPI info.version 1.1.0 → 1.2.0, with matching maxLength / maxItems.

How should this be tested?

  • go test ./cmd/api ./cmd/worker ./internal/...: pass
  • golangci-lint v2.14.0 run ./...: 0 issues; fmt --diff: clean
  • npx @stoplight/spectral-cli@6 lint openapi.yaml: no errors
  • The new tests fail against main's code: TestSearchHandler_SemanticSearch (over-long cases), TestTaxonomyLabelsAreBounded, TestRetryRefusesMoreNamesThanThereAreEnrichments, TestOpenAPIRequestArrayCapsMatchTheirValues. TestRespondErrorInvalidFieldTypeReasonTruncatesLongValue fails once the truncation in invalidFieldTypeParam is removed.
  • Not run locally: make tests (no local test DB) and Schemathesis. CI runs both.

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Repository Guidelines
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand bits
  • Ran make build
  • Ran make tests (integration tests in tests/)
  • Ran make fmt and make lint; no new warnings
  • Removed debug prints / temporary logging
  • Merged the latest changes from main onto my branch with git pull origin main
  • If database schema changed: added migration in migrations/ with goose annotations and ran make migrate-validate

Appreciated

  • If API changed: added or updated OpenAPI spec and ran contract tests (make tests or API contract workflow)
  • If API behavior changed: added request/response examples or Swagger UI screenshots to this PR
  • Updated docs in docs/ if changes were necessary
  • Ran make tests-coverage for meaningful logic changes

Semantic search query (4,096) and tenant_id (255), taxonomy run field_label and
node rename label (2,048), and enrichment retry names (3) get explicit limits,
mirrored in openapi.yaml (1.2.0). Webhook event_types gets maxItems/uniqueItems
in the spec, and a rejected field_type is quoted up to 64 characters.
@coderabbitai

coderabbitai Bot commented Oct 13, 2026

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 5cf9bb3e-9735-4a86-9b44-d3e728b0995f
📥 Commits

Reviewing files that changed from the base of the PR and between 541af90 and 6a24b7e.

📒 Files selected for processing (11)
  • internal/api/handlers/decode_json_handlers_test.go
  • internal/api/handlers/search_handler.go
  • internal/api/handlers/search_handler_test.go
  • internal/api/response/errors.go
  • internal/api/response/response_test.go
  • internal/models/feedback_records_filters_openapi_test.go
  • internal/models/request_arrays_openapi_test.go
  • internal/models/taxonomy.go
  • internal/service/enrichment_retry_service.go
  • internal/service/enrichment_retry_service_test.go
  • openapi.yaml

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The changes add maximum lengths for semantic-search fields and taxonomy labels, and maximum item counts for webhook event arrays and enrichment retry lists. Validation error reporting now truncates long invalid field-type values. Tests cover these limits and the related OpenAPI schemas.

Priority: ➖ Normal

Merge Risk: ⚪ Minimal · up to 6a24b

The request limits and webhook validation appear consistent. No issue requiring a fix before merge was identified.

Pre-merge checks · Passed 4 · Failed 1

Failed Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 10 files. (1 skipped:… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks (4 passed)
Check name Status Explanation
Description check Passed The description explains the changes, motivation, API behavior, testing performed, known test limitations, and checklist status.
Title check Passed The title follows Conventional Commits format and clearly summarizes the primary API change: bounding remaining unbounded request fields.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 10 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@pandeymangg
pandeymangg requested a review from xernobyl October 13, 2026 14:36

@xernobyl xernobyl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving. Checked against ENG-1652: every limit from the 10-13 status comment is in, in Go and in openapi.yaml, and info.version is bumped to 1.2.0 with a release note. metrics and the enrichment budget are explicitly out of scope (ENG-3908).

Smoke-tested the PR head against main on real Postgres with an embedding stub. Search query is 200 at 4,096 and 400 at 4,097 (characters, not bytes). Search tenant_id is 200 at 255 and 400 at 256. Taxonomy field_label and rename label are 200 at 2,048 and 400 at 2,049. Retry returns 400 for more than 3 names. A 400 KB field_type now returns about 480 bytes instead of 400 KB. Unit and integration tests, lint and Spectral all pass, and reverting each source change makes its new test fail.

Two optional follow-ups, neither blocking: nothing tests that the spec's maxLength values match the Go validators (only maxItems is pinned), and the docs don't mention the new limits (data-model.mdx lists the feedback-record ones).

@pandeymangg
pandeymangg added this pull request to the merge queue Oct 16, 2026
Merged via the queue into main with commit 7bea86a Oct 16, 2026
11 checks passed
@pandeymangg
pandeymangg deleted the anshuman/eng-1652-hub-request-field-limits branch October 16, 2026 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants