Skip to content

fix(api): cap translation request bodies#2203

Closed
tngus6007 wants to merge 1 commit into
Cap-go:mainfrom
tngus6007:harden-translation-request-body-limit
Closed

fix(api): cap translation request bodies#2203
tngus6007 wants to merge 1 commit into
Cap-go:mainfrom
tngus6007:harden-translation-request-body-limit

Conversation

@tngus6007
Copy link
Copy Markdown

@tngus6007 tngus6007 commented May 11, 2026

/claim #1667

Summary

  • Replace direct unbounded JSON body parsing on both translation message entrypoints with bounded stream reading.
  • Reject translation request bodies above 1 KiB with 413 request_body_too_large before cache, D1 storage, queue, or AI work.
  • Add regression coverage for streamed oversized bodies and Content-Length fast-fail handling in both translation implementations.

Security note

This is public-safe DoS hardening for the translation message endpoint. The accepted payload only needs targetLanguage, so the cap preserves normal use while avoiding unnecessary memory work on oversized request bodies.

Test plan

  • git diff --check
  • npx --yes bun@latest x vitest run tests/translation-public-body-limit.unit.test.ts tests/translation-queue.unit.test.ts returned exit code 0 in my local shell, but produced no stdout.

Screenshots

Not applicable; backend worker/API hardening only.

Checklist

  • Focused regression tests added
  • Public-safe security details only

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 11, 2026

Warning

Rate limit exceeded

@tngus6007 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 39 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 070f2f19-7312-4a28-91de-af57f0a05fef

📥 Commits

Reviewing files that changed from the base of the PR and between c06621a and 7c0c1b1.

📒 Files selected for processing (4)
  • cloudflare_workers/translation/index.ts
  • supabase/functions/_backend/public/translation.ts
  • tests/translation-public-body-limit.unit.test.ts
  • tests/translation-queue.unit.test.ts
📝 Walkthrough

Walkthrough

Adds a byte cap for translation POST bodies: a new MAX_TRANSLATION_REQUEST_BODY_BYTES constant, a stream-based readLimitedRequestBody() that returns HTTP 413 when exceeded, parseJsonBody() integration using the limiter, and two unit tests asserting 413 responses and no DB/queue interaction.

Changes

Request Body Size Limit

Layer / File(s) Summary
Limit Constant
cloudflare_workers/translation/index.ts
MAX_TRANSLATION_REQUEST_BODY_BYTES constant defines the maximum allowed request body size.
Stream-Based Limited Reader
cloudflare_workers/translation/index.ts
readLimitedRequestBody() reads request.body in chunks, enforces the byte cap, and triggers HTTP 413 when exceeded.
JSON Parsing Integration
cloudflare_workers/translation/index.ts
parseJsonBody() now accepts optional maxBytes (defaults to constant), uses the limited reader to obtain text, and preserves prior error handling (PublicHttpError rethrow; fallback to empty object on other parse/read errors).
Validation Tests
tests/translation-queue.unit.test.ts
Two tests added: one sends an oversized JSON payload, the other sends a large Content-Length with a stream; both expect 413 and verify no db.prepare, no queue.send, and (for the streamed test) that the stream pull is not invoked.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Cap-go/capgo#2046: Modifies the same translation worker request handling in cloudflare_workers/translation/index.ts.

Poem

I nibble bytes with careful cheer,
I cap the stream so nothing's drear,
When payloads swell beyond the door,
A gentle 413 stops the pour,
Rabbit hops off—queues untouched—🥕🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
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.
Description check ✅ Passed PR description covers all required template sections with clear security context, test plan, and completed checklist items.
Title check ✅ Passed The title 'fix(api): cap translation request bodies' clearly and concisely summarizes the main change—adding a size limit to translation request bodies—and is directly related to the primary modifications in the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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 and usage tips.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq Bot commented May 11, 2026

Merging this PR will not alter performance

✅ 43 untouched benchmarks
⏩ 2 skipped benchmarks1


Comparing tngus6007:harden-translation-request-body-limit (7c0c1b1) with main (ee54a9c)

Open in CodSpeed

Footnotes

  1. 2 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Copy link
Copy Markdown

@mingisrookie mingisrookie left a comment

Choose a reason for hiding this comment

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

This looks like a good focused hardening change. One small regression gap I'd consider before merging: the new helper has two separate enforcement paths, but the test only appears to exercise the streaming/read path where the body grows past the limit. The early Content-Length > MAX_TRANSLATION_REQUEST_BODY_BYTES fast-fail path is also security-relevant because it is the cheapest DoS rejection path and it happens before any body read.

If this branch can be reached in the Worker test harness, adding a case with an oversized Content-Length header and asserting the same 413 request_body_too_large / no db.prepare / no queue.send behavior would prevent a future refactor from accidentally dropping the pre-read rejection while leaving the streaming test green.

Copy link
Copy Markdown
Author

Added coverage for the Content-Length fast-fail path as requested. The new test asserts oversized declared length returns 413 request_body_too_large before the request stream is pulled, and before D1 storage or queue access.

Re-ran:

  • npx --yes bun@latest x vitest run tests/translation-queue.unit.test.ts
  • git diff --check

Copy link
Copy Markdown
Author

Also covered the existing /translation/messages API route imported by cloudflare_workers/api/index.ts, so both translation entrypoints now enforce the same 1 KiB request body cap. Added focused tests for the public API route's streamed overflow path and Content-Length fast-fail path.

Re-ran:

  • npx --yes bun@latest x vitest run tests/translation-public-body-limit.unit.test.ts tests/translation-queue.unit.test.ts
  • git diff --check

Copy link
Copy Markdown

@mingisrookie mingisrookie left a comment

Choose a reason for hiding this comment

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

Follow-up after re-running the requested coverage: the Content-Length behavior returns 413, but the new tests are currently failing locally because the stream pull spy is not stable evidence of route-level body consumption.

Comment thread tests/translation-public-body-limit.unit.test.ts Outdated
Comment thread tests/translation-queue.unit.test.ts Outdated
@sonarqubecloud
Copy link
Copy Markdown

@WcaleNieWolny
Copy link
Copy Markdown
Contributor

Closing as AI-generated spam. Part of a 50+ PR wave of duplicate redact logs PRs from disposable accounts. If you're human, open an issue first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants