Add condensed #inspect for Pool, Adapter, Config

Previously, it was very easy to accidentally leak a database password in
production logs if an error ends up calling inspect on a ConnectionPool
or an individual connection (Adapter). This is due to the default
`#inspect` output for Pools and Adapters being unnecessarily large, and
both currently including passwords (through the DatabaseConfig of a
Pool, and the internal configuration of an Adapter).

This commit addresses these issues by defining a custom `#inspect` for
ConnectionPool, AbstractAdapter, and DatabaseConfig. The condensed
`#inspect` only includes a few valuable fields instead of all of the
internals, which prevents both the large output and passwords from being
included.
This commit is contained in:
Hartley McGuire 2023-12-19 12:07:58 -05:00 committed by Rafael Mendonça França
parent fc60a394c0
commit 5c8172554f
No known key found for this signature in database
GPG Key ID: FC23B6D0F1EEE948
7 changed files with 44 additions and 0 deletions

@ -1,3 +1,8 @@
* Add condensed `#inspect` for `ConnectionPool`, `AbstractAdapter`, and
`DatabaseConfig`.
*Hartley McGuire*
* Add `.shard_keys`, `.sharded?`, & `.connected_to_all_shards` methods.
```ruby

@ -263,6 +263,13 @@ def initialize(pool_config)
@reaper.run
end
def inspect # :nodoc:
name_field = " name=#{db_config.name.inspect}" unless db_config.name == "primary"
shard_field = " shard=#{@shard.inspect}" unless @shard == :default
"#<#{self.class.name} env_name=#{db_config.env_name.inspect}#{name_field} role=#{role.inspect}#{shard_field}>"
end
def schema_cache
@schema_cache ||= BoundSchemaReflection.new(schema_reflection, self)
end

@ -172,6 +172,13 @@ def initialize(config_or_deprecated_connection, deprecated_logger = nil, depreca
@verified = false
end
def inspect # :nodoc:
name_field = " name=#{pool.db_config.name.inspect}" unless pool.db_config.name == "primary"
shard_field = " shard=#{shard.inspect}" unless shard == :default
"#<#{self.class.name}:#{'%#016x' % (object_id << 1)} env_name=#{pool.db_config.env_name.inspect}#{name_field} role=#{role.inspect}#{shard_field}>"
end
def lock_thread=(lock_thread) # :nodoc:
@lock =
case lock_thread

@ -18,6 +18,10 @@ def adapter_class
@adapter_class ||= ActiveRecord::ConnectionAdapters.resolve(adapter)
end
def inspect # :nodoc:
"#<#{self.class.name} env_name=#{@env_name} name=#{@name} adapter_class=#{adapter_class}>"
end
def new_connection
adapter_class.new(configuration_hash)
end

@ -323,6 +323,12 @@ def test_select_methods_passing_a_relation
test "type_to_sql returns a String for unmapped types" do
assert_equal "special_db_type", @connection.type_to_sql(:special_db_type)
end
test "inspect does not show secrets" do
output = @connection.inspect
assert_match(/ActiveRecord::ConnectionAdapters::\w+:0x[\da-f]+ env_name="\w+" role=:writing>/, output)
end
end
class AdapterForeignKeyTest < ActiveRecord::TestCase

@ -984,6 +984,16 @@ def test_pin_connection_nesting_lock_inverse
assert_equal ActiveSupport::Concurrency::NullLock, @pool.lease_connection.lock
end
def test_inspect_does_not_show_secrets
assert_match(/#<ActiveRecord::ConnectionAdapters::ConnectionPool env_name="\w+" role=:writing>/, @pool.inspect)
db_config = ActiveRecord::Base.connection_pool.db_config
pool_config = ActiveRecord::ConnectionAdapters::PoolConfig.new(ActiveRecord::Base, db_config, :reading, :shard_one)
pool = ConnectionPool.new(pool_config)
assert_match(/#<ActiveRecord::ConnectionAdapters::ConnectionPool env_name="\w+" role=:reading shard=:shard_one>/, pool.inspect)
end
private
def active_connections(pool)
pool.connections.find_all(&:in_use?)

@ -174,6 +174,11 @@ def test_validate_checks_the_adapter_exists
config.validate!
end
end
def test_inspect_does_not_show_secrets
config = HashConfig.new("default_env", "primary", { adapter: "abstract", password: "hunter2" })
assert_equal "#<ActiveRecord::DatabaseConfigurations::HashConfig env_name=default_env name=primary adapter_class=ActiveRecord::ConnectionAdapters::AbstractAdapter>", config.inspect
end
end
end
end