stop yaml_cache pollution

fixes CNVS-49087
flag = none

implement deep freezing
for the config values to
stop the pollution

dup all the locations
where the config is either
modified or passed off to
some other library where
it's not clear it was loaded
from a frozen config file

TEST PLAN:
 1) load a config file
 2) try to change it's key/vals
 3) you can't because it's frozen

Change-Id: I15faa230e3c99fe4806154493e238cecec526d1a
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/236341
Reviewed-by: James Williams <jamesw@instructure.com>
QA-Review: James Williams <jamesw@instructure.com>
Product-Review: James Williams <jamesw@instructure.com>
Tested-by: Service Cloud Jenkins <svc.cloudjenkins@instructure.com>

app/models/attachment.rb

@@ -89,11 +89,11 @@ class Attachment < ActiveRecord::Base
 
   def self.file_store_config
     # Return existing value, even if nil, as long as it's defined
-    @file_store_config ||= ConfigFile.load('file_store')
+    @file_store_config ||= ConfigFile.load('file_store').dup
     @file_store_config ||= { 'storage' => 'local' }
     @file_store_config['path_prefix'] ||= @file_store_config['path'] || 'tmp/files'
     @file_store_config['path_prefix'] = nil if @file_store_config['path_prefix'] == 'tmp/files' && @file_store_config['storage'] == 's3'
-    return @file_store_config
+    @file_store_config
   end
 
   def self.s3_config

config/initializers/delayed_job.rb

@@ -58,7 +58,7 @@ Delayed::Settings.default_job_options = ->{
 }
 
 # load our periodic_jobs.yml (cron overrides config file)
-Delayed::Periodic.add_overrides(ConfigFile.load('periodic_jobs') || {})
+Delayed::Periodic.add_overrides(ConfigFile.load('periodic_jobs').dup || {})
 
 if ActiveRecord::Base.configurations[Rails.env]['queue']
   ActiveSupport::Deprecation.warn("A queue section in database.yml is no longer supported. Please run migrations, then remove it.")

config/initializers/diigo.rb

@@ -16,5 +16,5 @@
 # with this program. If not, see <http://www.gnu.org/licenses/>.
 
 Diigo::Connection.config = Proc.new do
-  Canvas::Plugin.find(:diigo).try(:settings) || ConfigFile.load('diigo')
+  Canvas::Plugin.find(:diigo).try(:settings) || ConfigFile.load('diigo').dup
 end

config/initializers/google_drive.rb

@@ -21,5 +21,5 @@ GoogleDrive::Connection.config = proc do
     settings = settings.dup
     settings[:client_secret] = settings[:client_secret_dec]
   end
-  settings || ConfigFile.load('google_drive')
+  settings || ConfigFile.load('google_drive').dup
 end

config/initializers/incoming_mail.rb

@@ -17,7 +17,7 @@
 
 # Initialize incoming email configuration. See config/incoming_mail.yml.example.
 
-config = ConfigFile.load("incoming_mail") || {}
+config = ConfigFile.load("incoming_mail").dup || {}
 
 Rails.configuration.to_prepare do
   IncomingMailProcessor::IncomingMessageProcessor.configure(config)

config/initializers/inst_statsd.rb

@@ -19,7 +19,7 @@
 
 Rails.configuration.to_prepare do
   settings = YAML.safe_load(Canvas::DynamicSettings.find(tree: :private)['statsd.yml'] || '')
-  settings ||= ConfigFile.load("statsd")
+  settings ||= ConfigFile.load("statsd").dup
   settings ||= {}
   InstStatsd.settings = settings
 end

config/initializers/linked_in.rb

@@ -25,7 +25,7 @@ class CanvasLinkedInConfig
         secret_key: settings[:client_secret_dec]
       }.with_indifferent_access
     else
-      ConfigFile.load('linked_in')
+      ConfigFile.load('linked_in').dup
     end
 
   end

config/initializers/outgoing_mail.rb

@@ -24,7 +24,7 @@ require 'net/smtp'
 config = {
   :domain => "unknowndomain.example.com",
   :delivery_method => :smtp,
-}.merge((ConfigFile.load("outgoing_mail") || {}).symbolize_keys)
+}.merge((ConfigFile.load("outgoing_mail").dup || {}).symbolize_keys)
 
 [:authentication, :delivery_method].each do |key|
   config[key] = config[key].to_sym if config.has_key?(key)

config/initializers/session_store.rb

@@ -26,7 +26,7 @@ config = {
   :secret        => (Setting.get("session_secret_key", SecureRandom.hex(64), set_if_nx: true) rescue SecureRandom.hex(64)),
   legacy_key: '_legacy_normandy_session',
   same_site: :none
-}.merge((ConfigFile.load("session_store") || {}).symbolize_keys)
+}.merge((ConfigFile.load("session_store").dup || {}).symbolize_keys)
 
 # :expire_after is the "true" option, and :expires is a legacy option, but is applied
 # to the cookie after :expire_after is, so by setting it to nil, we force the lesser

config/initializers/twitter.rb

@@ -25,7 +25,7 @@ class CanvasTwitterConfig
         secret_key: settings[:consumer_secret_dec]
       }.with_indifferent_access
     else
-      ConfigFile.load('twitter')
+      ConfigFile.load('twitter').dup
     end
 
   end

lib/canvas.rb

@@ -39,7 +39,7 @@ module Canvas
     end
     @redis ||= begin
       Canvas::Redis.patch
-      settings = ConfigFile.load('redis')
+      settings = ConfigFile.load('redis').dup
       settings['url'] = settings['servers'] if settings['servers']
       ActiveSupport::Cache::RedisCacheStore.build_redis(settings.to_h.symbolize_keys)
     end

lib/canvas/dynamic_settings.rb

@@ -73,7 +73,7 @@ module Canvas
       end
 
       def root_fallback_proxy
-        @root_fallback_proxy ||= DynamicSettings::FallbackProxy.new(ConfigFile.load("dynamic_settings"))
+        @root_fallback_proxy ||= DynamicSettings::FallbackProxy.new(ConfigFile.load("dynamic_settings").dup)
       end
 
       # Build an object used to interacting with consul for the given

lib/config_file.rb

@@ -29,8 +29,9 @@ module ConfigFile
 
     def stub(config_name, value)
       raise "config settings can only be set via config file" unless Rails.env.test?
-      @yaml_cache[config_name] ||= {}
-      @yaml_cache[config_name][Rails.env] = value
+      existing_cache = @yaml_cache[config_name].dup || {}
+      existing_cache[Rails.env] = value
+      @yaml_cache[config_name] = deep_freeze_cached_value(existing_cache)
     end
 
     def load(config_name, with_rails_env = ::Rails.env)
@@ -43,13 +44,12 @@ module ConfigFile
       if File.exist?(path)
         config_string = ERB.new(File.read(path))
         config = YAML.load(config_string.result)
-
-        config = if config.respond_to?(:with_indifferent_access)
-                   config.with_indifferent_access
+        config = config.with_indifferent_access if config.respond_to?(:with_indifferent_access)
       end
+      if config
+        @yaml_cache[config_name] = deep_freeze_cached_value(config)
+        config = config[with_rails_env] if with_rails_env
       end
-      @yaml_cache[config_name] = config
-      config = config[with_rails_env] if config && with_rails_env
       config
     end
 
@@ -61,6 +61,29 @@ module ConfigFile
       config = load(config_name, with_rails_env)
       object_cache[with_rails_env] = config && yield(config)
     end
+
+    def deep_freeze_cached_value(input)
+      return nil if input.nil?
+      return deep_freeze_enumerable(input) if needs_deep_freeze?(input)
+      input.freeze
+      input
+    end
+
+    def deep_freeze_enumerable(input)
+      return nil if input.nil?
+      input.each do |key, value|
+        if input.is_a?(Array)
+          needs_deep_freeze?(key) ? deep_freeze_enumerable(key) : key.freeze
+        end
+        needs_deep_freeze?(value) ? deep_freeze_enumerable(value) : value.freeze
+      end
+      input.freeze
+      input
+    end
+
+    def needs_deep_freeze?(value)
+      value.is_a?(Enumerable) && !value.is_a?(String)
+    end
   end
   unstub
 end

lib/services/notification_service.rb

@@ -65,7 +65,7 @@ module Services
       end
 
       def config
-        ConfigFile.load('notification_service') || {}
+        ConfigFile.load('notification_service').dup || {}
       end
     end
   end

spec/lib/config_file_spec.rb

@@ -66,5 +66,40 @@ describe ConfigFile do
       expect(hit_block).to eq 2
       expect(result3).to be_nil
     end
+
+    it "does not give you the ability to mess with the cached data" do
+      ConfigFile.load("database", "test")
+      v2 = ConfigFile.load("database", "test")
+      expect { v2['foo'] = 'bar' }.to raise_error(RuntimeError)
+    end
+
+    describe "deep freezing" do
+      it "can deep freeze arrays" do
+        array = ["asdf","sdfg","dfgh","fghj"]
+        out = ConfigFile.deep_freeze_cached_value(array)
+        expect(out).to be_frozen
+        expect(out.class).to eq(Array)
+        expect(out[0]).to eq("asdf")
+        expect(out[0]).to be_frozen
+      end
+
+      it "can deep freeze hashes" do
+        hash = { "asdf" => "sdfg","dfgh" => "fghj" }
+        out = ConfigFile.deep_freeze_cached_value(hash)
+        expect(out).to be_frozen
+        expect(out.class).to eq(Hash)
+        expect(out["asdf"]).to be_frozen
+        expect(out["asdf"]).to eq("sdfg")
+      end
+
+      it "handles integers ok" do
+        array = [1,2,3,4]
+        out = ConfigFile.deep_freeze_cached_value(array)
+        expect(out).to be_frozen
+        expect(out.class).to eq(Array)
+        expect(out[0]).to eq(1)
+        expect(out[0]).to be_frozen
+      end
+    end
   end
 end

spec/models/attachment_spec.rb

@@ -31,6 +31,20 @@ describe Attachment do
 
   end
 
+  context "file_store_config" do
+    around(:each) do |example|
+      ConfigFile.unstub
+      example.run
+      ConfigFile.unstub
+    end
+
+    it "doesn't bomb on config" do
+      Attachment.instance_variable_set(:@file_store_config, nil)
+      ConfigFile.stub('file_store', { 'storage' => 'local' })
+      expect{ Attachment.file_store_config }.to_not raise_error
+    end
+  end
+
   context "default_values" do
     before :once do
       @course = course_model