From 508cd3e09011edb111467be9551b5a38e31737e0 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 21 Sep 2023 17:50:55 -0400 Subject: [PATCH 1/2] Redact password from query_options to avoid leaking credentials in exceptions via #inspect Closes #1049 --- lib/mysql2/client.rb | 4 ++++ spec/mysql2/client_spec.rb | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/lib/mysql2/client.rb b/lib/mysql2/client.rb index 582b6e305..485158ada 100644 --- a/lib/mysql2/client.rb +++ b/lib/mysql2/client.rb @@ -78,6 +78,10 @@ def initialize(opts = {}) warn "============= END WARNING FROM mysql2 =========" end + # avoid logging sensitive data via #inspect + @query_options.delete(:password) + @query_options.delete(:pass) + user = opts[:username] || opts[:user] pass = opts[:password] || opts[:pass] host = opts[:host] || opts[:hostname] diff --git a/spec/mysql2/client_spec.rb b/spec/mysql2/client_spec.rb index 14f446df9..0698cb600 100644 --- a/spec/mysql2/client_spec.rb +++ b/spec/mysql2/client_spec.rb @@ -1173,4 +1173,23 @@ 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 From 89f486c9e6416c5fe3c53b242488ef63914d3bb0 Mon Sep 17 00:00:00 2001 From: Mike Dalessio Date: Thu, 28 Sep 2023 10:09:00 -0400 Subject: [PATCH 2/2] Address rubocop complaints --- .rubocop_todo.yml | 2 +- lib/mysql2/client.rb | 26 ++++++++++++++++---------- spec/mysql2/client_spec.rb | 3 +-- spec/mysql2/statement_spec.rb | 2 +- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index eba2091dc..02cfbae17 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -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. diff --git a/lib/mysql2/client.rb b/lib/mysql2/client.rb index 485158ada..a1e7961be 100644 --- a/lib/mysql2/client.rb +++ b/lib/mysql2/client.rb @@ -71,16 +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 - - # avoid logging sensitive data via #inspect - @query_options.delete(:password) - @query_options.delete(:pass) + check_and_clean_query_options user = opts[:username] || opts[:user] pass = opts[:password] || opts[:pass] @@ -169,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 diff --git a/spec/mysql2/client_spec.rb b/spec/mysql2/client_spec.rb index 0698cb600..44f6ab699 100644 --- a/spec/mysql2/client_spec.rb +++ b/spec/mysql2/client_spec.rb @@ -1176,8 +1176,7 @@ def run_gc it "should not include the password in the output of #inspect" do client_class = Class.new(Mysql2::Client) do - def connect(*args) - end + def connect(*args); end end client = client_class.new(password: "secretsecret") diff --git a/spec/mysql2/statement_spec.rb b/spec/mysql2/statement_spec.rb index 57e590804..c3ebf9bb3 100644 --- a/spec/mysql2/statement_spec.rb +++ b/spec/mysql2/statement_spec.rb @@ -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