Skip to content
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

Ensure YAML safe loading in Rails 6.1 #1399

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 22 additions & 4 deletions lib/paper_trail/serializers/yaml.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def load(string)
if use_safe_load?
::YAML.safe_load(
string,
permitted_classes: ::ActiveRecord.yaml_column_permitted_classes,
permitted_classes: yaml_column_permitted_classes,
aliases: true
)
elsif ::YAML.respond_to?(:unsafe_load)
Expand All @@ -39,10 +39,28 @@ def where_object_condition(arel_field, field, value)

private

# `use_yaml_unsafe_load` was added in 7.0.3.1, will be removed in 7.1.0?
def use_safe_load?
defined?(ActiveRecord.use_yaml_unsafe_load) &&
!ActiveRecord.use_yaml_unsafe_load
if ::ActiveRecord.gem_version >= Gem::Version.new("7.0.3.1")
# `use_yaml_unsafe_load` may be removed in the future, at which point safe loading will be
# the default.
!defined?(ActiveRecord.use_yaml_unsafe_load) || !ActiveRecord.use_yaml_unsafe_load
elsif defined?(ActiveRecord::Base.use_yaml_unsafe_load)
# Rails 5.2.8.1, 6.0.5.1, 6.1.6.1
!ActiveRecord::Base.use_yaml_unsafe_load
else
false
end
end

def yaml_column_permitted_classes
if ::ActiveRecord.gem_version >= Gem::Version.new("7.0.3.1")
ActiveRecord.yaml_column_permitted_classes
elsif defined?(ActiveRecord::Base.yaml_column_permitted_classes)
# Rails 5.2.8.1, 6.0.5.1, 6.1.6.1
ActiveRecord::Base.yaml_column_permitted_classes
else
[]
end
end
end
end
Expand Down
31 changes: 18 additions & 13 deletions spec/dummy_app/config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

module Dummy
class Application < Rails::Application
config.load_defaults(::Rails.gem_version.segments.take(2).join("."))
config.load_defaults(::ActiveRecord.gem_version.segments.take(2).join("."))

config.encoding = "utf-8"
config.filter_parameters += [:password]
Expand All @@ -24,22 +24,27 @@ class Application < Rails::Application
# In rails >= 6.0, "`.represent_boolean_as_integer=` is now always true,
# so setting this is deprecated and will be removed in Rails 6.1."
if ::ENV["DB"] == "sqlite" &&
::Gem::Requirement.new("~> 5.2").satisfied_by?(::Rails.gem_version)
::Gem::Requirement.new("~> 5.2").satisfied_by?(::ActiveRecord.gem_version)
config.active_record.sqlite3.represent_boolean_as_integer = true
end

# `use_yaml_unsafe_load` was added in 7.0.3.1, will be removed in 7.1.0?
if ::ActiveRecord.respond_to?(:use_yaml_unsafe_load)
YAML_COLUMN_PERMITTED_CLASSES = [
::ActiveRecord::Type::Time::Value,
::ActiveSupport::TimeWithZone,
::ActiveSupport::TimeZone,
::BigDecimal,
::Date,
::Symbol,
::Time
].freeze

# `use_yaml_unsafe_load` was added in 5.2.8.1, 6.0.5.1, 6.1.6.1, and 7.0.3.1
if ::ActiveRecord.gem_version >= Gem::Version.new("7.0.3.1")
::ActiveRecord.use_yaml_unsafe_load = false
::ActiveRecord.yaml_column_permitted_classes = [
::ActiveRecord::Type::Time::Value,
::ActiveSupport::TimeWithZone,
::ActiveSupport::TimeZone,
::BigDecimal,
::Date,
::Symbol,
::Time
]
::ActiveRecord.yaml_column_permitted_classes = YAML_COLUMN_PERMITTED_CLASSES
elsif ::ActiveRecord::Base.respond_to?(:use_yaml_unsafe_load)
::ActiveRecord::Base.use_yaml_unsafe_load = false
::ActiveRecord::Base.yaml_column_permitted_classes = YAML_COLUMN_PERMITTED_CLASSES
end
end
end
16 changes: 13 additions & 3 deletions spec/paper_trail/serializers/yaml_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ module Serializers
end

it "calls the expected load method based on Psych version" do
# `use_yaml_unsafe_load` was added in 7.0.3.1, will be removed in 7.1.0?
if defined?(ActiveRecord.use_yaml_unsafe_load) && !ActiveRecord.use_yaml_unsafe_load
# `use_yaml_unsafe_load` was added in 5.2.8.1, 6.0.5.1, 6.1.6.1, and 7.0.3.1
if rails_supports_safe_load?
allow(::YAML).to receive(:safe_load)
described_class.load("string")
expect(::YAML).to have_received(:safe_load)
# Psych 4+ implements .unsafe_load
# Psych 4+ implements .unsafe_load
elsif ::YAML.respond_to?(:unsafe_load)
allow(::YAML).to receive(:unsafe_load)
described_class.load("string")
Expand Down Expand Up @@ -60,6 +60,16 @@ module Serializers
expect(arel_value(matches.right)).to eq("%\narg1: Val 1\n%")
end
end

private

def rails_supports_safe_load?
# Rails 7.0.3.1 onwards will always support YAML safe loading
return true if ::ActiveRecord.gem_version >= Gem::Version.new("7.0.3.1")

# Older Rails versions may or may not, depending on whether they have been patched.
defined?(ActiveRecord::Base.use_yaml_unsafe_load)
end
end
end
end