feat: add rate limiting to /invite endpoint#439
feat: add rate limiting to /invite endpoint#439mrigangha wants to merge 1 commit intonetlify:masterfrom
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe changes add rate limiting to the Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.11.4)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
api/invite_test.go (1)
10-33:⚠️ Potential issue | 🟠 MajorReplace
os.Setenvwitht.Setenvand remove the unusedosimport.Line 33 uses
os.Setenv()which mutates process-wide state without cleanup, creating test isolation issues and test order dependencies. Uset.Setenv()instead—it automatically restores the environment variable after the test completes.Suggested fix
import ( "bytes" "encoding/json" "fmt" "net/http" "net/http/httptest" "net/url" - "os" "testing" "time" @@ func TestInvite(t *testing.T) { - os.Setenv("GOTRUE_RATE_LIMIT_HEADER", "My-Custom-Header") + t.Setenv("GOTRUE_RATE_LIMIT_HEADER", "My-Custom-Header") api, config, instanceID, err := setupAPIForTestForInstance()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@api/invite_test.go` around lines 10 - 33, Replace the process-wide os.Setenv call in the TestInvite test with t.Setenv to ensure the environment variable is scoped to the test and automatically restored; update the TestInvite function to call t.Setenv("GOTRUE_RATE_LIMIT_HEADER", "My-Custom-Header") instead of os.Setenv, and remove the now-unused os import from the invite_test.go top imports so there are no unused import errors; locate the TestInvite function and the InviteTestSuite type to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@api/invite_test.go`:
- Around line 10-33: Replace the process-wide os.Setenv call in the TestInvite
test with t.Setenv to ensure the environment variable is scoped to the test and
automatically restored; update the TestInvite function to call
t.Setenv("GOTRUE_RATE_LIMIT_HEADER", "My-Custom-Header") instead of os.Setenv,
and remove the now-unused os import from the invite_test.go top imports so there
are no unused import errors; locate the TestInvite function and the
InviteTestSuite type to apply the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: efba17e7-2760-4c1b-8d63-e0177561d406
📒 Files selected for processing (2)
api/api.goapi/invite_test.go
- Summary
The
/inviteendpoint had no rate limiting despite being an email-sendingendpoint protected by
requireAdminCredentials. A compromised admin tokencould be used to send bulk invite emails with no throttling. This PR wires
a tollbooth limiter to
/invitematching the pattern already used by/token.Closes #413
- Test plan
Added
TestInviteRateLimitwhich sends 10 requests (exhausting the burst)and asserts the 11th returns
429 Too Many Requests. All existing invitetests continue to pass.
make test 2>&1 | grep -E "TestInvite"
--- PASS: TestInvite/TestInvite (0.13s)
--- PASS: TestInvite/TestInviteExternalGitlab (0.14s)
--- PASS: TestInvite/TestInviteExternalGitlab_MismatchedEmails (0.13s)
--- PASS: TestInvite/TestInviteRateLimit (0.67s)
--- PASS: TestInvite/TestInvite_WithoutAccess (0.07s)
--- PASS: TestInvite/TestVerifyInvite (0.17s)
--- PASS: TestInvite/TestVerifyInvite_NoPassword (0.12s)
- Description for the changelog
Add tollbooth rate limiting to the /invite endpoint (10 req/min, burst of 10)
- A picture of a cute animal (not mandatory but encouraged)
🐾