fix: rate limiting on public API endpoints - #2
Open
devbydaniel wants to merge 8 commits into
Open
devbydaniel wants to merge 8 commits into
devbydaniel wants to merge 8 commits into
Conversation
…kens - Updated RaterLimiter interface to return (remaining float64, error) - Modified Bucket.consume() to return remaining token count - Updated TokenBucketRateLimit.Deduct() to return remaining tokens - Returns remaining count even when rate limit is reached
Created GetClientIP function to extract client IP addresses from HTTP requests with proper proxy header support. Implementation: - Checks X-Forwarded-For header first (for proxy/load balancer support) - Falls back to X-Real-IP header - Falls back to RemoteAddr with port stripping - Handles both IPv4 and IPv6 addresses correctly - Properly handles edge cases (multiple IPs, whitespace, missing ports) Added comprehensive test coverage with 11 test cases covering: - Header precedence (X-Forwarded-For > X-Real-IP > RemoteAddr) - IPv4 and IPv6 address handling - Multiple IP parsing (takes first from X-Forwarded-For) - Edge cases (whitespace, empty headers, no port) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add RateLimitPublicByIP middleware for IP-based rate limiting - Add RateLimitPublicByOrg middleware for org-based rate limiting - Initialize separate rate limiters using config values - Add X-RateLimit-Limit, X-RateLimit-Remaining headers to responses - Return HTTP 429 with X-RateLimit-Retry-After when limit exceeded - Follow patterns from auth.go for logging and error handling Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Created comprehensive integration test suite for rate limiting - test_rate_limiting.sh: Automated test script for IP-based and org-based rate limiting - RATE_LIMITING_TESTS.md: Complete documentation and manual testing guide Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
…e config, fix race condition (qa-requested) Fixes: - Issue #1: Fixed inverted cost calculation formula (PublicMaxTokens / PublicRequestsPerInterval) - Issue #2: Cache config in init() instead of creating on every request - Issue #3: Add mutex to protect buckets map from concurrent access - Issue #4: Add config validation for rate limit values Verified: - Cost calculation now correctly limits requests to configured value - Config loaded only once at startup instead of per-request - Map access protected with sync.RWMutex and double-check pattern - Invalid config values (<=0) will panic at startup with clear error QA Fix Session: 1 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fixes:
- Pointer to local variable causing undefined behavior (critical)
- Documentation inconsistency in RATE_LIMITING_TESTS.md (minor)
Changes:
1. backend/internal/ratelimit/ratelimit.go:64-65
- Changed 'bucket := Bucket{...}' to 'newBucket := &Bucket{...}'
- Prevents dangling pointer by creating Bucket on heap
- Eliminates variable shadowing that caused memory safety bug
2. backend/RATE_LIMITING_TESTS.md:123-130
- Updated environment variable names to match .env.example
- Removed '_PER_INTERVAL' suffixes
- Changed '_INTERVAL_SECONDS' to '_REFILL_SECONDS'
Verified:
- Code review confirms no variable shadowing
- Environment variable names match actual implementation
QA Fix Session: 2
Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add rate limiting to all public-facing API endpoints (widget config, release notes listing, metrics tracking, like endpoints) to prevent abuse and ensure service stability.