Skip to content

Commit 354c54e

Browse files
committed
Always wrap passwords in Proc so they're not exposed through inspect
Followup: redis/redis-rb#1299 Also applies to sentinel passwords.
1 parent 6c415cc commit 354c54e

File tree

5 files changed

+50
-43
lines changed

5 files changed

+50
-43
lines changed

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# Unreleased
22

3+
- Allow `sentinel_password` to be provided as a `Proc`.
4+
- Ensure `Config#inspect` and `Config#to_s` do not display stored passwords.
5+
36
# 0.23.2
47

58
- Fix retry logic not to attempt to retry on an open circuit breaker. Fix #227.

lib/redis_client/config.rb

Lines changed: 27 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ def initialize(
4040
circuit_breaker: nil
4141
)
4242
@username = username
43-
@password = password
43+
@password = password && !password.respond_to?(:call) ? ->(_) { password } : password
4444
@db = begin
4545
Integer(db || DEFAULT_DB)
4646
rescue ArgumentError
@@ -70,7 +70,6 @@ def initialize(
7070

7171
reconnect_attempts = Array.new(reconnect_attempts, 0).freeze if reconnect_attempts.is_a?(Integer)
7272
@reconnect_attempts = reconnect_attempts
73-
@connection_prelude = (build_connection_prelude unless @password.respond_to?(:call))
7473

7574
circuit_breaker = CircuitBreaker.new(**circuit_breaker) if circuit_breaker.is_a?(Hash)
7675
if @circuit_breaker = circuit_breaker
@@ -88,19 +87,36 @@ def initialize(
8887
end
8988

9089
def connection_prelude
91-
if @password.respond_to?(:call)
92-
build_connection_prelude
93-
else
94-
@connection_prelude
90+
prelude = []
91+
pass = password
92+
if protocol == 3
93+
prelude << if pass
94+
["HELLO", "3", "AUTH", username, pass]
95+
else
96+
["HELLO", "3"]
97+
end
98+
elsif pass
99+
prelude << if @username && !@username.empty?
100+
["AUTH", @username, pass]
101+
else
102+
["AUTH", pass]
103+
end
95104
end
105+
106+
if @db && @db != 0
107+
prelude << ["SELECT", @db.to_s]
108+
end
109+
110+
# Deep freeze all the strings and commands
111+
prelude.map! do |commands|
112+
commands = commands.map { |str| str.frozen? ? str : str.dup.freeze }
113+
commands.freeze
114+
end
115+
prelude.freeze
96116
end
97117

98118
def password
99-
if @password.respond_to?(:call)
100-
@password.call(username)
101-
else
102-
@password
103-
end
119+
@password&.call(username)
104120
end
105121

106122
def username
@@ -162,37 +178,6 @@ def server_url
162178
end
163179
url
164180
end
165-
166-
private
167-
168-
def build_connection_prelude
169-
prelude = []
170-
pass = password
171-
if protocol == 3
172-
prelude << if pass
173-
["HELLO", "3", "AUTH", username, pass]
174-
else
175-
["HELLO", "3"]
176-
end
177-
elsif pass
178-
prelude << if @username && !@username.empty?
179-
["AUTH", @username, pass]
180-
else
181-
["AUTH", pass]
182-
end
183-
end
184-
185-
if @db && @db != 0
186-
prelude << ["SELECT", @db.to_s]
187-
end
188-
189-
# Deep freeze all the strings and commands
190-
prelude.map! do |commands|
191-
commands = commands.map { |str| str.frozen? ? str : str.dup.freeze }
192-
commands.freeze
193-
end
194-
prelude.freeze
195-
end
196181
end
197182

198183
include Common

lib/redis_client/sentinel_config.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,10 @@ def initialize(
3838
end
3939

4040
@to_list_of_hash = @to_hash = nil
41+
password = sentinel_password && !sentinel_password.respond_to?(:call) ? ->(_) { sentinel_password } : sentinel_password
4142
@extra_config = {
4243
username: sentinel_username,
43-
password: sentinel_password,
44+
password: password,
4445
db: nil,
4546
}
4647
if client_config[:protocol] == 2

test/redis_client/config_test.rb

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,18 @@ def test_resp3_user_password_uri
8686
assert_equal [%w[HELLO 3 AUTH username password]], config.connection_prelude
8787
end
8888

89+
def test_hide_password
90+
config = Config.new(password: "PASSWORD")
91+
refute_match "PASSWORD", config.inspect
92+
refute_match "PASSWORD", config.to_s
93+
end
94+
95+
def test_hide_password_in_uri
96+
config = Config.new(url: "redis://username:[email protected]")
97+
refute_match "PASSWORD", config.inspect
98+
refute_match "PASSWORD", config.to_s
99+
end
100+
89101
def test_resp2_frozen_prelude
90102
config = Config.new(protocol: 2, url: "redis://username:[email protected]")
91103
prelude = config.connection_prelude

test/sentinel/sentinel_test.rb

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,12 @@ def test_sentinel_refresh_password
257257
assert_equal 1, sentinel_client_mock.close_count
258258
end
259259

260+
def test_hide_sentinel_password
261+
config = new_config(sentinel_password: "PASSWORD")
262+
refute_match "PASSWORD", config.inspect
263+
refute_match "PASSWORD", config.to_s
264+
end
265+
260266
def test_config_user_password_from_url_for_redis_master_replica_only
261267
config = new_config(url: "redis://george:hunter2@cache/10", name: nil)
262268
assert_equal "hunter2", config.password

0 commit comments

Comments
 (0)