20300: Call safe_load explicitly instead of using safe_yaml gem.

safe_yaml breaks new i18n.

ArgumentError: wrong number of arguments (given 2, expected 1)
/home/tom/.gem/ruby/2.7.0/gems/safe_yaml-1.0.5/lib/safe_yaml.rb:37:in `unsafe_load_file'
/home/tom/.gem/ruby/2.7.0/gems/i18n-1.14.1/lib/i18n/backend/base.rb:254:in `load_yml'
...

Arvados-DCO-1.1-Signed-off-by: Tom Clegg <tom@curii.com>

diff --git a/services/api/Gemfile b/services/api/Gemfile
index b2b6d1fbc40c7c9f85b802efdc1f74171ae8d545..d9ac19dd0838bb5adf5080b45b540d64d8fe0542 100644 (file)
--- a/services/api/Gemfile
+++ b/services/api/Gemfile
@@ -53,7 +53,6 @@ gem 'themes_for_rails', git: 'https://github.com/arvados/themes_for_rails'
 gem 'arvados', '~> 2.7.0.rc1'
 gem 'httpclient'
 
-gem 'safe_yaml'
 gem 'lograge'
 gem 'logstash-event'
 

diff --git a/services/api/Gemfile.lock b/services/api/Gemfile.lock
index d7f1938372230595357f721ae93e047b8388511b..1aedac90305f359cb45ccbe92ef87d8b91d0ed50 100644 (file)
--- a/services/api/Gemfile.lock
+++ b/services/api/Gemfile.lock
@@ -176,13 +176,13 @@ GEM
     nokogiri (1.15.4)
       mini_portile2 (~> 2.8.2)
       racc (~> 1.4)
-    oj (3.16.0)
+    oj (3.16.1)
     optimist (3.1.0)
     os (1.1.4)
     passenger (6.0.18)
       rack
       rake (>= 0.8.1)
-    pg (1.5.3)
+    pg (1.5.4)
     power_assert (2.0.3)
     public_suffix (5.0.3)
     racc (1.7.1)
@@ -236,7 +236,6 @@ GEM
     retriable (1.4.1)
     ruby-prof (0.15.9)
     ruby2_keywords (0.0.5)
-    safe_yaml (1.0.5)
     signet (0.16.1)
       addressable (~> 2.8)
       faraday (>= 0.17.5, < 3.0)
@@ -297,7 +296,6 @@ DEPENDENCIES
   rails-perftest
   responders
   ruby-prof (~> 0.15.0)
-  safe_yaml
   simplecov (~> 0.7.1)
   simplecov-rcov
   sprockets (~> 3.0)

diff --git a/services/api/app/controllers/database_controller.rb b/services/api/app/controllers/database_controller.rb
index 6bcbd52798308ab24197356d1f735a8f701f420c..38d406fe333f785c5dbf218c5d5d84a04986e49e 100644 (file)
--- a/services/api/app/controllers/database_controller.rb
+++ b/services/api/app/controllers/database_controller.rb
@@ -19,8 +19,8 @@ class DatabaseController < ApplicationController
       where('email is null or (email not like ? and email not like ?)', '%@example.com', '%.example.com').
       collect(&:uuid)
     fixture_uuids =
-      YAML::load_file(File.expand_path('../../../test/fixtures/users.yml',
-                                       __FILE__)).
+      YAML::safe_load_file(File.expand_path('../../../test/fixtures/users.yml',
+                                            __FILE__)).
       values.collect { |u| u['uuid'] }
     unexpected_uuids = user_uuids - fixture_uuids
     if unexpected_uuids.any?

diff --git a/services/api/app/models/workflow.rb b/services/api/app/models/workflow.rb
index 94890c6632e3b9a8ea6eadf914edd9552ec618e5..0268c4e9797195c79668bd9b6b16244468f3502d 100644 (file)
--- a/services/api/app/models/workflow.rb
+++ b/services/api/app/models/workflow.rb
@@ -18,7 +18,7 @@ class Workflow < ArvadosModel
 
   def validate_definition
     begin
-      @definition_yaml = YAML.load self.definition if !definition.nil?
+      @definition_yaml = YAML.safe_load self.definition if !definition.nil?
     rescue => e
       errors.add :definition, "is not valid yaml: #{e.message}"
     end
@@ -27,7 +27,7 @@ class Workflow < ArvadosModel
   def set_name_and_description
     old_wf = {}
     begin
-      old_wf = YAML.load self.definition_was if !self.definition_was.nil?
+      old_wf = YAML.safe_load self.definition_was if !self.definition_was.nil?
     rescue => e
       logger.warn "set_name_and_description error: #{e.message}"
       return

diff --git a/services/api/config/application.rb b/services/api/config/application.rb
index b28ae0e0718e2ddabc472f61be8cf8c07a53232f..dcd77d7b6b85c2bf4e3204343f8ccab912364dd5 100644 (file)
--- a/services/api/config/application.rb
+++ b/services/api/config/application.rb
@@ -47,8 +47,6 @@ end
 
 module Server
   class Application < Rails::Application
-    # The following is to avoid SafeYAML's warning message
-    SafeYAML::OPTIONS[:default_mode] = :safe
 
     require_relative "arvados_config.rb"
 

diff --git a/services/api/config/arvados_config.rb b/services/api/config/arvados_config.rb
index d928d592c93f7b01f58145a37d08b380941d1720..f8b9ff8ecdd650ceb0a556565fd600001087168d 100644 (file)
--- a/services/api/config/arvados_config.rb
+++ b/services/api/config/arvados_config.rb
@@ -36,7 +36,7 @@ if !status.success?
   puts stderr
   raise "error loading config: #{status}"
 end
-confs = YAML.load(defaultYAML, deserialize_symbols: false)
+confs = YAML.safe_load(defaultYAML)
 clusterID, clusterConfig = confs["Clusters"].first
 $arvados_config_defaults = clusterConfig
 $arvados_config_defaults["ClusterID"] = clusterID
@@ -50,7 +50,7 @@ if ENV["ARVADOS_CONFIG"] == "none"
 else
   # Load the global config file
   Open3.popen2("arvados-server", "config-dump", "-skip-legacy") do |stdin, stdout, status_thread|
-    confs = YAML.load(stdout, deserialize_symbols: false)
+    confs = YAML.safe_load(stdout)
     if confs && !confs.empty?
       # config-dump merges defaults with user configuration, so every
       # key should be set.
@@ -198,7 +198,7 @@ application_config = {}
   path = "#{::Rails.root.to_s}/config/#{cfgfile}.yml"
   confs = ConfigLoader.load(path, erb: true)
   # Ignore empty YAML file:
-  next if confs == false
+  next if confs == nil
   application_config.deep_merge!(confs['common'] || {})
   application_config.deep_merge!(confs[::Rails.env.to_s] || {})
 end

diff --git a/services/api/config/initializers/reload_config.rb b/services/api/config/initializers/reload_config.rb
index b54e3bcf87e764aec3508aec0ff5444cc7de9c37..22eee1601bb43940de5254c6cb10205181975d03 100644 (file)
--- a/services/api/config/initializers/reload_config.rb
+++ b/services/api/config/initializers/reload_config.rb
@@ -29,7 +29,7 @@ def start_reload_thread
         # precision cannot represent multiple updates per second.
         if t.to_f != t_lastload.to_f || Time.now.to_f - t.to_f < 5
           Open3.popen2("arvados-server", "config-dump", "-skip-legacy") do |stdin, stdout, status_thread|
-            confs = YAML.load(stdout, deserialize_symbols: false)
+            confs = YAML.safe_load(stdout)
             hash = confs["SourceSHA256"]
           rescue => e
             Rails.logger.info("reload_config: config file updated but could not be loaded: #{e}")

diff --git a/services/api/lib/config_loader.rb b/services/api/lib/config_loader.rb
index f421fb5b2a07817905c28bbd1a463da9be936ac5..1d897b39bf4de9af98b4710035199ce831a97d5c 100644 (file)
--- a/services/api/lib/config_loader.rb
+++ b/services/api/lib/config_loader.rb
@@ -2,6 +2,16 @@
 #
 # SPDX-License-Identifier: AGPL-3.0
 
+# When loading YAML, deserialize :foo as ":foo", rather than raising
+# "Psych::DisallowedClass: Tried to load unspecified class: Symbol"
+class Psych::ScalarScanner
+  alias :orig_tokenize :tokenize
+  def tokenize string
+    return string if string =~ /^:[a-zA-Z]/
+    orig_tokenize(string)
+  end
+end
+
 module Psych
   module Visitors
     class YAMLTree < Psych::Visitors::Visitor
@@ -226,7 +236,7 @@ class ConfigLoader
       if erb
         yaml = ERB.new(yaml).result(binding)
       end
-      YAML.load(yaml, deserialize_symbols: false)
+      YAML.safe_load(yaml)
     else
       {}
     end

diff --git a/services/api/lib/migrate_yaml_to_json.rb b/services/api/lib/migrate_yaml_to_json.rb
index 1db7ed0113e27cf8fad51e5c83a2d7153f5ccca0..aa2af60b25552187776ef4c797598d0990990b32 100644 (file)
--- a/services/api/lib/migrate_yaml_to_json.rb
+++ b/services/api/lib/migrate_yaml_to_json.rb
@@ -19,7 +19,7 @@ module MigrateYAMLToJSON
         [[nil, '---%']],
       ).rows.map do |id, yaml|
         n += 1
-        json = SafeJSON.dump(YAML.load(yaml))
+        json = SafeJSON.dump(YAML.safe_load(yaml))
         conn.exec_query(
           "UPDATE #{table} SET #{column}=$1 WHERE id=$2 AND #{column}=$3",
           "#{table}.#{column} convert YAML to JSON",

diff --git a/services/api/lib/serializers.rb b/services/api/lib/serializers.rb
index 37734e0bb41dce88500b143fab0a71102b1b8b33..c25b9060b4100871e2ec832e318120829db9ef4e 100644 (file)
--- a/services/api/lib/serializers.rb
+++ b/services/api/lib/serializers.rb
@@ -16,7 +16,7 @@ class Serializer
   end
 
   def self.legacy_load(s)
-    val = Psych.safe_load(s)
+    val = Psych.safe_load(s, permitted_classes: [Time])
     if val.is_a? String
       # If apiserver was downgraded to a YAML-only version after
       # storing JSON in the database, the old code would have loaded

diff --git a/services/api/script/arvados-git-sync.rb b/services/api/script/arvados-git-sync.rb
index ad6aaf9eb567205498a5e5c1b8dd4b61202e0f52..ceebc3518a08d22586a4f6ee2b184f1d90744253 100755 (executable)
--- a/services/api/script/arvados-git-sync.rb
+++ b/services/api/script/arvados-git-sync.rb
@@ -26,7 +26,7 @@ DEBUG = 1
 # if present, overriding base config parameters as specified
 path = File.absolute_path('../../config/arvados-clients.yml', __FILE__)
 if File.exist?(path) then
-  cp_config = YAML.load_file(path)[ENV['RAILS_ENV']]
+  cp_config = YAML.safe_load_file(path)[ENV['RAILS_ENV']]
 else
   puts "Please create a\n #{path}\n file"
   exit 1

diff --git a/services/api/script/migrate-gitolite-to-uuid-storage.rb b/services/api/script/migrate-gitolite-to-uuid-storage.rb
index 91acf3e256c96ced3262f946e36c37868507b1b2..d2b9a0418b71aaf1e132d0b1b22c48d33af18950 100755 (executable)
--- a/services/api/script/migrate-gitolite-to-uuid-storage.rb
+++ b/services/api/script/migrate-gitolite-to-uuid-storage.rb
@@ -40,7 +40,7 @@ DEBUG = 1
 # if present, overriding base config parameters as specified
 path = File.dirname(__FILE__) + '/config/arvados-clients.yml'
 if File.exist?(path) then
-  cp_config = YAML.load_file(path)[ENV['RAILS_ENV']]
+  cp_config = YAML.safe_load_file(path)[ENV['RAILS_ENV']]
 else
   puts "Please create a\n " + File.dirname(__FILE__) + "/config/arvados-clients.yml\n file"
   exit 1