Skip to content

fix: rate limit resumable upload endpoints#79

Merged
PranavAgarkar07 merged 1 commit into
PranavAgarkar07:mainfrom
mittalsonal:codex/beamsync-resumable-rate-limit-51
Jun 17, 2026
Merged

fix: rate limit resumable upload endpoints#79
PranavAgarkar07 merged 1 commit into
PranavAgarkar07:mainfrom
mittalsonal:codex/beamsync-resumable-rate-limit-51

Conversation

@mittalsonal

@mittalsonal mittalsonal commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Summary

  • apply the existing upload rate limiter to /upload/resumable
  • apply the same limiter to /upload-status/
  • add a regression test proving both resumable endpoints return 429 after the upload limit is exceeded

Validation

  • git diff --check

Note: Go tooling is not installed in this environment (gofmt/go are unavailable on PATH), so I could not run gofmt or go test locally.

Closes #51

Summary by CodeRabbit

  • Bug Fixes

    • Rate limiting is now enforced on resumable upload endpoints. Requests exceeding the configured limit will receive a 429 (Too Many Requests) response.
  • Tests

    • Added tests to verify rate limiting behavior on resumable upload endpoints.

Signed-off-by: Garima Varshney <gungunvarshney00@gmail.com>
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: fc251add-9c9b-49dd-911c-6f3d5b037fa8

📥 Commits

Reviewing files that changed from the base of the PR and between be465d0 and 249ae49.

📒 Files selected for processing (2)
  • beamsync/server.go
  • beamsync/server_test.go

📝 Walkthrough

Walkthrough

rateLimitMiddleware using uploadLimiter is added to the /upload/resumable and /upload-status/ route registrations in server.go, mirroring the existing protection on the regular /upload endpoint. A new test TestResumableEndpointsAreRateLimited verifies the 429 response and Retry-After header after the limit is hit.

Changes

Resumable Upload Rate Limiting

Layer / File(s) Summary
Apply rateLimitMiddleware to resumable routes
beamsync/server.go, beamsync/server_test.go
/upload/resumable and /upload-status/ are now wrapped with rateLimitMiddleware(uploadLimiter, ...) in addition to tokenMiddleware. TestResumableEndpointsAreRateLimited issues 12 baseline requests then one extra, asserting HTTP 429 and a non-empty Retry-After header for both endpoints.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PranavAgarkar07/BeamSync#33: Introduced the resumable upload endpoints (/upload/resumable and /upload-status/) whose middleware wiring is directly modified by this PR.
  • PranavAgarkar07/BeamSync#25: Introduced rateLimitMiddleware and the 429/Retry-After behavior that this PR extends to the resumable routes.

Suggested labels

type:security

🐇 Hoppity hop, the chunks won't flood,
A rate-limit shield where once there stood
Only a token, thin and bare—
Now 429 fills the air! 🚫
No more disk storms, no chunk despair,
The bunny guards each upload with care. 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding rate limiting to resumable upload endpoints.
Linked Issues check ✅ Passed The PR fully addresses all coding objectives from issue #51: rate limiting applied to both endpoints and regression test validates 429 responses.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the rate limiting requirements; no unrelated modifications detected.

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

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

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.12.2)

level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies"


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.

@PranavAgarkar07 PranavAgarkar07 added gssoc:approved Approved for GSSoC contributions level:beginner Beginner friendly task type:security Security labels Jun 17, 2026
@PranavAgarkar07 PranavAgarkar07 merged commit f0006b5 into PranavAgarkar07:main Jun 17, 2026
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gssoc:approved Approved for GSSoC contributions level:beginner Beginner friendly task type:security Security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🔒 [Security] Resumable upload endpoints missing rate limiting

3 participants