Skip to content

Commit

Permalink
Redact password from query_options (#1334)
Browse files Browse the repository at this point in the history
Redact password from query_options to avoid leaking
credentials in exceptions via #inspect. Minor refactor
to make RuboCop happy about client.rb complexity.

Closes #1049
  • Loading branch information
flavorjones authored Dec 3, 2024
1 parent f0c5d11 commit 03ac203
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 8 deletions.
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ Metrics/BlockNesting:
# Offense count: 1
# Configuration parameters: CountComments, CountAsOne.
Metrics/ClassLength:
Max: 135
Max: 141

# Offense count: 3
# Configuration parameters: IgnoredMethods.
Expand Down
22 changes: 16 additions & 6 deletions lib/mysql2/client.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,12 +71,7 @@ def initialize(opts = {})
# SSL verify is a connection flag rather than a mysql_ssl_set option
flags |= SSL_VERIFY_SERVER_CERT if opts[:sslverify]

if %i[user pass hostname dbname db sock].any? { |k| @query_options.key?(k) }
warn "============= WARNING FROM mysql2 ============="
warn "The options :user, :pass, :hostname, :dbname, :db, and :sock are deprecated and will be removed at some point in the future."
warn "Instead, please use :username, :password, :host, :port, :database, :socket, :flags for the options."
warn "============= END WARNING FROM mysql2 ========="
end
check_and_clean_query_options

user = opts[:username] || opts[:user]
pass = opts[:password] || opts[:pass]
Expand Down Expand Up @@ -165,6 +160,21 @@ def info
self.class.info
end

private

def check_and_clean_query_options
if %i[user pass hostname dbname db sock].any? { |k| @query_options.key?(k) }
warn "============= WARNING FROM mysql2 ============="
warn "The options :user, :pass, :hostname, :dbname, :db, and :sock are deprecated and will be removed at some point in the future."
warn "Instead, please use :username, :password, :host, :port, :database, :socket, :flags for the options."
warn "============= END WARNING FROM mysql2 ========="
end

# avoid logging sensitive data via #inspect
@query_options.delete(:password)
@query_options.delete(:pass)
end

class << self
private

Expand Down
18 changes: 18 additions & 0 deletions spec/mysql2/client_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1195,4 +1195,22 @@ def run_gc
it "should respond to #encoding" do
expect(@client).to respond_to(:encoding)
end

it "should not include the password in the output of #inspect" do
client_class = Class.new(Mysql2::Client) do
def connect(*args); end
end

client = client_class.new(password: "secretsecret")

expect(client.inspect).not_to include("password")
expect(client.inspect).not_to include("secretsecret")

expect do
client = client_class.new(pass: "secretsecret")
end.to output(/WARNING/).to_stderr

expect(client.inspect).not_to include("pass")
expect(client.inspect).not_to include("secretsecret")
end
end
2 changes: 1 addition & 1 deletion spec/mysql2/statement_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ def stmt_count
result = @client.prepare("SELECT 1 UNION SELECT 2").execute(stream: true, cache_rows: false)
expect do
result.each {}
result.each {} # rubocop:disable Style/CombinableLoops
result.each {}
end.to raise_exception(Mysql2::Error)
end
end
Expand Down

0 comments on commit 03ac203

Please sign in to comment.