chore: downgrade redis to 5.4.1 - #4
Conversation
…ck cache store initialization
WalkthroughThe Redis dependency now targets the ChangesRedis cache handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@config/initializers/rack_attack.rb`:
- Around line 9-17: Update the Rack::Attack cache-store initialization around
ActiveSupport::Cache::RedisCacheStore to probe Redis with a ping and configure
an error_handler that raises cache errors in production while warning in
non-production. In the rescue path, retain MemoryStore only for non-production
environments and re-raise the exception in production, preserving the existing
warning context.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1db2fa8a-30c1-45ab-80fd-77137a76b78c
⛔ Files ignored due to path filters (1)
Gemfile.lockis excluded by!**/*.lock,!Gemfile.lock
📒 Files selected for processing (2)
Gemfileconfig/initializers/rack_attack.rb
| begin | ||
| ActiveSupport::Cache::RedisCacheStore.new( | ||
| url: redis_url, | ||
| namespace: 'rack_attack' | ||
| ) | ||
| rescue StandardError => e | ||
| Rails.logger.warn("Rack::Attack: Redis failed (#{e.message}), using MemoryStore") | ||
| ActiveSupport::Cache::MemoryStore.new | ||
| end |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
: "${REDIS_URL:?Set REDIS_URL to a disposable Redis endpoint}"
ruby -e '
require "active_support/cache/redis_cache_store"
store = ActiveSupport::Cache::RedisCacheStore.new(
url: ENV.fetch("REDIS_URL"),
namespace: "rack_attack"
)
abort "Redis client lacks `#with`" unless store.redis.respond_to?(:with)
store.redis.with(&:ping)
puts "Redis connection succeeded"
'Repository: Euler-B/Telurify-API
Length of output: 228
🌐 Web query:
Rails 7.2.3 ActiveSupport::Cache::RedisCacheStore error_handler redis with ping source
💡 Result:
In Rails 7.2.3, the ActiveSupport::Cache::RedisCacheStore uses a default error handler defined as DEFAULT_ERROR_HANDLER [1][2]. This handler is responsible for logging errors via the Rails logger and reporting them to the ActiveSupport error reporter with the source "redis_cache_store.active_support" [1][2]. The default error handler is defined as: DEFAULT_ERROR_HANDLER = -> (method:, returning:, exception:) do if logger logger.error { "RedisCacheStore: #{method} failed, returned #{returning.inspect}: #{exception.class}: #{exception.message}" } end ActiveSupport.error_reporter&.report( exception, severity::warning, source: "redis_cache_store.active_support",) end The RedisCacheStore internal implementation includes a failsafe method that wraps cache operations and calls this @error_handler if a Redis::BaseError, ConnectionPool::Error, or ConnectionPool::TimeoutError occurs [2]. Regarding your mention of "ping": ActiveSupport::Cache::RedisCacheStore does not include a specific "ping" option or native mechanism within its error handler configuration to automatically ping Redis upon an error [1][2]. If you need to perform health checks or pings, you would typically implement that logic within a custom error handler provided to the RedisCacheStore during initialization [1][2]. You can override the default behavior by passing a custom error_handler proc when creating your RedisCacheStore instance: # Example of providing a custom error handler config.cache_store =:redis_cache_store, { error_handler: -> (method:, returning:, exception:) do # Add your custom logic here, such as logging, metrics, or pinging Redis Rails.logger.error("Redis error in #{method}: #{exception.message}") end } If you are encountering specific errors, ensure they are among those caught by the store's internal failsafe mechanism (Redis::BaseError, ConnectionPool::Error, or ConnectionPool::TimeoutError) [2].
Citations:
- 1: https://api.rubyonrails.org/v7.2.3/classes/ActiveSupport/Cache/RedisCacheStore.html
- 2: https://github.com/rails/rails/blob/v7.2.3.1/activesupport/lib/active_support/cache/redis_cache_store.rb
🌐 Web query:
Rack::Attack cache store increment nil behavior source
💡 Result:
In Rack::Attack, the cache store's increment method is handled with a fallback mechanism to accommodate differences in how various cache stores (such as Redis or Memcached) behave when incrementing a key that does not yet exist [1][2]. The relevant logic is located in the Rack::Attack::Cache#do_count method within the lib/rack/attack/cache.rb source file [1][2]. When Rack::Attack attempts to increment a counter, it calls store.increment and checks the result [1][2]: 1. Detection of Nil: Some cache stores return nil when an attempt is made to increment a key that has not been initialized [1][2]. 2. Fallback Logic: If the result of the increment call is nil, Rack::Attack immediately executes store.write(key, 1, expires_in: expires_in) to manually initialize the key with a value of 1 [1][2]. 3. Return Value: The method ultimately returns result || 1, ensuring that the application receives a valid integer (typically 1 for a new entry) even if the underlying store returned nil [1][2]. This implementation is designed to handle potential inconsistencies across different ActiveSupport::Cache stores, ensuring that rate limiting counters remain accurate even when the cache backend does not support atomic "increment-or-initialize" behavior [1][2].
Citations:
- 1: https://github.com/rack/rack-attack/blob/main/lib/rack/attack/cache.rb
- 2: https://github.com/kickstarter/rack-attack/blob/master/lib/rack/attack/cache.rb
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
python3 - <<'PY'
from urllib.request import urlopen
urls = {
"redis_cache_store": "https://raw.githubusercontent.com/rails/rails/v7.2.3/activesupport/lib/active_support/cache/redis_cache_store.rb",
"rack_attack_cache": "https://raw.githubusercontent.com/rack/rack-attack/main/lib/rack/attack/cache.rb",
}
for name, url in urls.items():
print(f"--- {name} ---")
text = urlopen(url, timeout=10).read().decode()
lines = text.splitlines()
for i, line in enumerate(lines, 1):
if any(token in line for token in ("def failsafe", "error_handler", "def increment", "result || 1", "def do_count")):
lo, hi = max(1, i - 8), min(len(lines), i + 22)
print("\n".join(f"{n}: {lines[n-1]}" for n in range(lo, hi + 1)))
print()
PYRepository: Euler-B/Telurify-API
Length of output: 2470
Rate-limit Failure-open (CWE-799)
Reachability: External · Exploitability: Moderate
Make Redis failure handling fail closed for rate limiting.
MemoryStore is process-local, so clustered Puma workers do not share fallback counters. Later Redis failures also return nil; Rack Attack converts that result to count 1, which disables throttling.
Use MemoryStore only outside production, probe Redis during initialization, and raise on production cache errors:
begin
store = ActiveSupport::Cache::RedisCacheStore.new(
url: redis_url,
namespace: 'rack_attack',
error_handler: lambda do |method:, returning:, exception:|
raise exception if Rails.env.production?
Rails.logger.warn("Rack::Attack: Redis #{method} failed")
end
)
store.redis.with(&:ping)
store
rescue StandardError => e
Rails.logger.warn("Rack::Attack: Redis failed (#{e.message}), using MemoryStore")
raise if Rails.env.production?
ActiveSupport::Cache::MemoryStore.new
end🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@config/initializers/rack_attack.rb` around lines 9 - 17, Update the
Rack::Attack cache-store initialization around
ActiveSupport::Cache::RedisCacheStore to probe Redis with a ping and configure
an error_handler that raises cache errors in production while warning in
non-production. In the rescue path, retain MemoryStore only for non-production
environments and re-raise the exception in production, preserving the existing
warning context.
Source: Path instructions
1 participant
Summary by CodeRabbit