Limit JSON payload and register token size#27
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Rate limit exceeded
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 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 configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
AndreaDiazCorreia
left a comment
There was a problem hiding this comment.
Good job but Before merging: the 413 body comes back as text/plain and echoes the exact server limit (JSON payload (X bytes) is larger than allowed (limit: Y bytes)), which both breaks the JSON everywhere shape and gives free fingerprinting, so please add an error_handler on both JsonConfigs returning something like {"success":false,"message":"Payload too large"}. Also missing a test for the 413 path on /api/notify covering registered vs unregistered plus x-request-id presence, which matters given that endpoint's privacy contract. Optional nits: 128 KiB on /api/register is generous (16 to 32 KiB would do the same job with less parse surface), and JSON_PAYLOAD_LIMIT_BYTES could be renamed DEFAULT_JSON_PAYLOAD_LIMIT_BYTES for clarity.
Summary
Security validation
Source/sink/control: attacker-controlled JSON bodies and register token values reached Actix JSON extraction and token storage without explicit size controls. The app now configures bounded JSON extraction and rejects register tokens above 4096 bytes before storage.
Original issue no longer reproduces:
Refs #8
Tests