Security: Auto-detect Redis for rate limiting storage (#446)#498
Merged
filthyrake merged 2 commits intodevfrom Jan 4, 2026
Merged
Security: Auto-detect Redis for rate limiting storage (#446)#498filthyrake merged 2 commits intodevfrom
filthyrake merged 2 commits intodevfrom
Conversation
Addresses the security issue where in-memory rate limiting allows attackers to bypass rate limits in multi-instance deployments by distributing requests across instances. Changes: - Auto-detect Redis: If VLOG_REDIS_URL is configured, rate limiting now automatically uses Redis storage instead of in-memory - Explicit override: VLOG_RATE_LIMIT_STORAGE_URL still takes precedence if explicitly set - Enhanced warnings: Startup warnings now explicitly mention "SECURITY" and explain the attack vector - Updated documentation: CONFIGURATION.md now has security warnings and explains auto-detection - Updated .env.example: Better comments explaining the security issue and auto-detection feature This is a non-breaking change. Existing deployments continue to work: - Single instance: Defaults to memory:// (with warning) - Multi-instance with Redis: Automatically uses Redis - Explicit config: Honors VLOG_RATE_LIMIT_STORAGE_URL Fixes #446 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changes based on specialist code review: 1. Add security warning to worker_api.py (was missing) - Matches admin.py and public.py warnings 2. Improve config.py clarity: - Better variable names: _explicit_rate_limit_storage, _redis_url_for_rate_limit - Avoid redundant os.getenv() calls - Add info log when Redis is auto-detected (aids debugging) 3. Add unit tests for auto-detection logic (tests/test_config.py): - test_defaults_to_memory_when_no_redis - test_auto_detects_redis_from_redis_url - test_explicit_storage_takes_precedence_over_redis_url - test_explicit_memory_overrides_redis_url - test_empty_redis_url_falls_back_to_memory 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 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.
Summary
Addresses issue #446: Rate limiting defaults to in-memory storage, which is a security vulnerability in multi-instance deployments.
VLOG_REDIS_URLis configured, rate limiting now automatically uses Redis storageVLOG_RATE_LIMIT_STORAGE_URLstill takes precedenceTest plan
VLOG_REDIS_URLis set (uses Redis)VLOG_RATE_LIMIT_STORAGE_URLtakes precedenceBreaking changes
None. This is backwards compatible:
🤖 Generated with Claude Code