Strictly check the type loaded from YAML

Feedback:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/8088#note_20062470

Eventually we should move to SafeYAML, but requiring that
would impact all other `YAML.load` which is bad. For this
particular case, I think we could just check it strictly.

lib/gitlab/serialize/yaml_variables.rb

@@ -10,15 +10,36 @@ module Gitlab
       def load(string)
         return unless string
 
-        YAML.load(string).
-          map(&YamlVariables.method(:convert_key_value_to_string))
+        object = YAML.load(string)
+
+        # We don't need to verify the object once we're using SafeYAML
+        if YamlVariables.verify_object(object)
+          YamlVariables.convert_object(object)
+        else
+          []
+        end
       end
 
       def dump(object)
         YAML.dump(object)
       end
 
-      private
+      def verify_object(object)
+        YamlVariables.verify_type(object, Array) &&
+          object.all? { |obj| YamlVariables.verify_type(obj, Hash) }
+      end
+
+      # We use three ways to check if the class is exactly the one we want,
+      # rather than some subclass or duck typing class.
+      def verify_type(object, klass)
+        object.kind_of?(klass) &&
+          object.class == klass &&
+          klass === object
+      end
+
+      def convert_object(object)
+        object.map(&YamlVariables.method(:convert_key_value_to_string))
+      end
 
       def convert_key_value_to_string(variable)
         variable[:key] = variable[:key].to_s

spec/lib/gitlab/serialize/yaml_variables_spec.rb

@@ -16,4 +16,25 @@ describe Gitlab::Serialize::YamlVariables do
       { key: 'key', value: 'value', public: true },
       { key: 'wee', value: '1', public: false }])
   end
+
+  context 'with a subclass of Array' do
+    let(:object) do
+      Kaminari::PaginatableArray.new << 'I am evil'
+    end
+
+    it 'ignores it' do
+      is_expected.to eq([])
+    end
+  end
+
+  context 'with the array containing subclasses of Hash' do
+    let(:object) do
+      [ActiveSupport::OrderedOptions.new(
+        key: 'key', value: 'value', public: true)]
+    end
+
+    it 'ignores it' do
+      is_expected.to eq([])
+    end
+  end
 end