Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -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.
Expand Down
69 changes: 27 additions & 42 deletions lib/redis_client/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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 && [email protected]?
["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
Expand Down Expand Up @@ -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 && [email protected]?
["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
Expand Down
7 changes: 6 additions & 1 deletion lib/redis_client/sentinel_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 12 additions & 0 deletions test/redis_client/config_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions test/sentinel/sentinel_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down