Merge pull request #230 from redis-rb/hide-password

Always wrap passwords in Proc so they're not exposed through `inspect`

Author: byroot

CHANGELOG.md

@@ -1,5 +1,8 @@
 # Unreleased
 
+- Allow `sentinel_password` to be provided as a `Proc`.
+- Ensure `Config#inspect` and `Config#to_s` do not display stored passwords.
+
 # 0.23.2
 
 - Fix retry logic not to attempt to retry on an open circuit breaker. Fix #227.

lib/redis_client/config.rb

@@ -40,7 +40,7 @@ def initialize(
         circuit_breaker: nil
       )
         @username = username
-        @password = password
+        @password = password && !password.respond_to?(:call) ? ->(_) { password } : password
         @db = begin
           Integer(db || DEFAULT_DB)
         rescue ArgumentError
@@ -70,7 +70,6 @@ def initialize(
 
         reconnect_attempts = Array.new(reconnect_attempts, 0).freeze if reconnect_attempts.is_a?(Integer)
         @reconnect_attempts = reconnect_attempts
-        @connection_prelude = (build_connection_prelude unless @password.respond_to?(:call))
 
         circuit_breaker = CircuitBreaker.new(**circuit_breaker) if circuit_breaker.is_a?(Hash)
         if @circuit_breaker = circuit_breaker
@@ -88,19 +87,36 @@ def initialize(
       end
 
       def connection_prelude
-        if @password.respond_to?(:call)
-          build_connection_prelude
-        else
-          @connection_prelude
+        prelude = []
+        pass = password
+        if protocol == 3
+          prelude << if pass
+            ["HELLO", "3", "AUTH", username, pass]
+          else
+            ["HELLO", "3"]
+          end
+        elsif pass
+          prelude << if @username && !@username.empty?
+            ["AUTH", @username, pass]
+          else
+            ["AUTH", pass]
+          end
         end
+
+        if @db && @db != 0
+          prelude << ["SELECT", @db.to_s]
+        end
+
+        # Deep freeze all the strings and commands
+        prelude.map! do |commands|
+          commands = commands.map { |str| str.frozen? ? str : str.dup.freeze }
+          commands.freeze
+        end
+        prelude.freeze
       end
 
       def password
-        if @password.respond_to?(:call)
-          @password.call(username)
-        else
-          @password
-        end
+        @password&.call(username)
       end
 
       def username
@@ -162,37 +178,6 @@ def server_url
         end
         url
       end
-
-      private
-
-      def build_connection_prelude
-        prelude = []
-        pass = password
-        if protocol == 3
-          prelude << if pass
-            ["HELLO", "3", "AUTH", username, pass]
-          else
-            ["HELLO", "3"]
-          end
-        elsif pass
-          prelude << if @username && !@username.empty?
-            ["AUTH", @username, pass]
-          else
-            ["AUTH", pass]
-          end
-        end
-
-        if @db && @db != 0
-          prelude << ["SELECT", @db.to_s]
-        end
-
-        # Deep freeze all the strings and commands
-        prelude.map! do |commands|
-          commands = commands.map { |str| str.frozen? ? str : str.dup.freeze }
-          commands.freeze
-        end
-        prelude.freeze
-      end
     end
 
     include Common

lib/redis_client/sentinel_config.rb

@@ -38,9 +38,14 @@ def initialize(
       end
 
       @to_list_of_hash = @to_hash = nil
+      password = if sentinel_password && !sentinel_password.respond_to?(:call)
+        ->(_) { sentinel_password }
+      else
+        sentinel_password
+      end
       @extra_config = {
         username: sentinel_username,
-        password: sentinel_password,
+        password: password,
         db: nil,
       }
       if client_config[:protocol] == 2

test/redis_client/config_test.rb

@@ -86,6 +86,18 @@ def test_resp3_user_password_uri
       assert_equal [%w[HELLO 3 AUTH username password]], config.connection_prelude
     end
 
+    def test_hide_password
+      config = Config.new(password: "PASSWORD")
+      refute_match "PASSWORD", config.inspect
+      refute_match "PASSWORD", config.to_s
+    end
+
+    def test_hide_password_in_uri
+      config = Config.new(url: "redis://username:[email protected]")
+      refute_match "PASSWORD", config.inspect
+      refute_match "PASSWORD", config.to_s
+    end
+
     def test_resp2_frozen_prelude
       config = Config.new(protocol: 2, url: "redis://username:[email protected]")
       prelude = config.connection_prelude

test/sentinel/sentinel_test.rb

@@ -257,6 +257,12 @@ def test_sentinel_refresh_password
       assert_equal 1, sentinel_client_mock.close_count
     end
 
+    def test_hide_sentinel_password
+      config = new_config(sentinel_password: "PASSWORD")
+      refute_match "PASSWORD", config.inspect
+      refute_match "PASSWORD", config.to_s
+    end
+
     def test_config_user_password_from_url_for_redis_master_replica_only
       config = new_config(url: "redis://george:hunter2@cache/10", name: nil)
       assert_equal "hunter2", config.password