Skip to content

Commit 79658d7

Browse files
pftgclaude
andcommitted
Fix CRITICAL production safety issues from CodeRabbit PR #266
CRITICAL FIXES (Priority 1 - Production Safety): 1. Docker dev stack target mismatch (rails-8-docker-deployment-production-guide.md) - Changed web and worker services from 'target: production' to 'target: base' - Fixes: Dev environment building production image breaks hot-reload/tooling - Impact: Development workflow now uses correct base image 2. Invalid docker-compose rollback command (rails-8-docker-deployment-production-guide.md) - Replaced non-existent 'docker-compose rollback' with tag-based rollback - Fixes: Rollback procedure that wouldn't work in production - Impact: Production rollback now uses proper docker tag reversion 3. SECURITY: Credentials leak via redirect params (rails-8-authentication-generator-devise-migration.md) - Changed redirect_to rails8_session_path(params: params) to only forward email - Fixes: Passwords/emails exposed in URL/logs during authentication redirect - Impact: Prevents credential leakage in production logs and URLs 4. Rake task mutates production passwords (rails-8-authentication-generator-devise-migration.md) - Replaced password mutation with read-only digest format validation - Fixes: Task permanently corrupts production user credentials - Impact: Validation now checks digest format without writing user data 5. Cache warmer uses KEYS blocking Redis (rails-8-solid-cache-performance-redis-migration.md) - Replaced redis.keys('*') with cursor-based SCAN batching - Fixes: KEYS command blocks Redis in production during cache warming - Impact: Non-blocking cache warming via SCAN iteration 6. uses_redis_specific_features? uses KEYS (rails-8-solid-cache-performance-redis-migration.md) - Replaced redis.keys('*') with cursor-based SCAN iteration - Fixes: KEYS command blocks Redis during feature detection - Impact: Non-blocking feature detection via SCAN Tests: All critical tests passing (bin/rake test:critical) Reference: CodeRabbit PR #266 review feedback Methodology: XP pair programming (coder + reviewer) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 87cba8e commit 79658d7

3 files changed

Lines changed: 61 additions & 32 deletions

content/blog/2025/rails-8-authentication-generator-devise-migration.md

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -819,7 +819,8 @@ class SessionsController < ApplicationController
819819

820820
if user && AuthMigration.use_rails8_auth?(user)
821821
# Redirect to Rails 8 authentication
822-
redirect_to rails8_session_path(params: params)
822+
# SECURITY: Never forward raw params (contains password)
823+
redirect_to rails8_session_path(email: params[:email])
823824
else
824825
# Use Devise authentication
825826
super
@@ -904,15 +905,17 @@ namespace :auth do
904905
password_compatibility: 0
905906
}
906907

907-
# Test password compatibility
908+
# Test password compatibility (read-only validation)
908909
User.limit(100).each do |user|
909910
next unless user.encrypted_password.present?
910911

911-
test_password = SecureRandom.hex(16)
912-
user.update(password: test_password, password_confirmation: test_password)
913-
914-
if user.authenticate(test_password) && user.valid_password?(test_password)
915-
checks[:password_compatibility] += 1
912+
# Validate digest format without mutating user data
913+
if user.password_digest.present? && user.encrypted_password.present?
914+
# Check that both digests exist and are properly formatted
915+
if BCrypt::Password.valid_hash?(user.password_digest) &&
916+
user.encrypted_password.start_with?('$2a$')
917+
checks[:password_compatibility] += 1
918+
end
916919
end
917920
end
918921

content/blog/2025/rails-8-docker-deployment-production-guide.md

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ services:
311311
build:
312312
context: .
313313
dockerfile: Dockerfile
314-
target: production # Use production image
314+
target: base # Use base image for development hot-reload
315315
command: bundle exec rails server -b 0.0.0.0
316316
volumes:
317317
# Mount code for development hot-reload
@@ -343,7 +343,7 @@ services:
343343
build:
344344
context: .
345345
dockerfile: Dockerfile
346-
target: production
346+
target: base # Use base image for development consistency
347347
command: bundle exec rails solid_queue:start
348348
volumes:
349349
- .:/rails
@@ -552,7 +552,10 @@ echo "💚 Performing health check..."
552552
sleep 10
553553
curl -f http://localhost/up || {
554554
echo "❌ Health check failed! Rolling back..."
555-
docker-compose -f docker-compose.production.yml rollback
555+
# Rollback by reverting to previous image tag
556+
docker-compose -f docker-compose.production.yml down
557+
docker tag myapp/rails:previous myapp/rails:latest
558+
docker-compose -f docker-compose.production.yml up -d
556559
exit 1
557560
}
558561

content/blog/2025/rails-8-solid-cache-performance-redis-migration.md

Lines changed: 45 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -334,8 +334,21 @@ class RedisCacheAudit
334334
end
335335

336336
def self.uses_redis_specific_features?
337-
# Check for sorted sets, pub/sub, etc.
338-
Redis.current.keys('*').any? { |k| Redis.current.type(k) != 'string' }
337+
# Check for sorted sets, pub/sub, etc. using SCAN (non-blocking)
338+
redis = Redis.current
339+
cursor = "0"
340+
341+
loop do
342+
cursor, keys = redis.scan(cursor, count: 100)
343+
344+
keys.each do |key|
345+
return true if redis.type(key) != 'string'
346+
end
347+
348+
break if cursor == "0"
349+
end
350+
351+
false
339352
end
340353
end
341354
```
@@ -478,33 +491,43 @@ class CacheWarmer
478491
redis = Redis.new(url: ENV['REDIS_URL'])
479492
solid_cache = Rails.cache
480493

481-
# Get all cache keys from Redis
482-
keys = redis.keys('*')
483-
484-
puts "Warming #{keys.size} cache entries..."
485-
486-
keys.each_slice(1000).with_index do |batch, index|
487-
ActiveRecord::Base.transaction do
488-
batch.each do |key|
489-
# Read from Redis
490-
value = redis.get(key)
491-
ttl = redis.ttl(key)
494+
# Use SCAN instead of KEYS to avoid blocking Redis
495+
cursor = "0"
496+
total_keys = 0
497+
batch_count = 0
492498

493-
next unless value
499+
puts "Starting cache warming with SCAN batching..."
494500

495-
# Write to Solid Cache with same TTL
496-
solid_cache.write(
497-
key,
498-
value,
499-
expires_in: ttl > 0 ? ttl.seconds : nil
500-
)
501+
loop do
502+
cursor, keys = redis.scan(cursor, count: 1000)
503+
total_keys += keys.size
504+
505+
unless keys.empty?
506+
ActiveRecord::Base.transaction do
507+
keys.each do |key|
508+
# Read from Redis
509+
value = redis.get(key)
510+
ttl = redis.ttl(key)
511+
512+
next unless value
513+
514+
# Write to Solid Cache with same TTL
515+
solid_cache.write(
516+
key,
517+
value,
518+
expires_in: ttl > 0 ? ttl.seconds : nil
519+
)
520+
end
501521
end
522+
523+
batch_count += 1
524+
puts "Processed batch #{batch_count} (#{total_keys} keys total)"
502525
end
503526

504-
puts "Processed batch #{index + 1} of #{keys.size / 1000}"
527+
break if cursor == "0"
505528
end
506529

507-
puts "Cache warming complete!"
530+
puts "Cache warming complete! Warmed #{total_keys} entries."
508531
end
509532

510533
def self.verify_warmup

0 commit comments

Comments
 (0)